-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support webpack #37
base: master
Are you sure you want to change the base?
support webpack #37
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, we need to be sure that the result will still work for our umd/commonjs users
src/WebLinq.ts
Outdated
@@ -0,0 +1,5 @@ | |||
import * as LinqCollection from "./Linq"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this file.
Webpack has the option to target UMD and it does this automatically
webpack.config.js
Outdated
mode, | ||
entry: { | ||
"linq-collection": [ | ||
'./src/WebLinq.ts' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 versions: node (commonjs) and web (umd)
webpack.config.js
Outdated
|
||
module.exports = [ | ||
getConfig("production", "bundle.min"), | ||
getConfig("development", "bundle") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we need 'development'
package.json
Outdated
"publish-node": "npm run node-compile", | ||
"publish-web": "browserify ./src/Linq.ts -p [tsify] > ./build/src/linq.web.js", | ||
"prepublish": "npm run node-compile" | ||
"build-web": "webpack-cli", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"build-web" -> "build"
this is triggered by continuous integration service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, change "typings" and "main" to point dist/linq-collections.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should include the whole "dist" folder to the repo (with the newest compiled version)
package.json
Outdated
"exclude": [ | ||
"build/test" | ||
"**/*.test.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line not needed
No description provided.