chore(build): Switch to rollup + Update Dependencies #150
Conversation
Updates all the dependencies and changes the build from webpack to rollup exposing now multiple formats: - umd - umd minified - cjs - es
|
This is awesome @deini ! Let me know if there is any more work you want to do on this, if not, we can just merge it in. |
|
Oh, I meant to ask! @deini is the |
|
@deini sounds good! I trust Kyle's opinion on these matters. :) |
| .DS_Store | ||
| *.log | ||
| *.tgs | ||
| *.yml |
There was a problem hiding this comment.
quick question about this, why aren't any of these being ignored any longer?
There was a problem hiding this comment.
I added files in package.json which pretty much instead of having to ignore what we don't want, we explicitly add what we do want. Therefore no need to have .npmignore anymore.
There was a problem hiding this comment.
Had no idea about that one! Thanks! :)
package.json
Outdated
| "build:umd:min": "cross-env NODE_ENV=production rollup -c -o dist/ng-redux.min.js", | ||
| "build": "yarn run build:commonjs && yarn run build:es && yarn run build:umd && yarn run build:umd:min", | ||
| "test": "NODE_ENV=test mocha --compilers js:babel-register --recursive", | ||
| "prepublish": "yarn run clean && yarn test && yarn run build" |
There was a problem hiding this comment.
@deini why did we switch to using yarn over npm in the build process? We don't have yarn in the dependencies, and I, for example, don't have yarn on my machine because it's still screwy in some ways and not as cross-system compatible (Windows definitely suffers here).
There was a problem hiding this comment.
Good point, switched to $npm_execpath
package.json
Outdated
| "build:es": "cross-env NODE_ENV=es rollup -c -o es/ng-redux.js", | ||
| "build:umd": "cross-env NODE_ENV=development rollup -c -o dist/ng-redux.js", | ||
| "build:umd:min": "cross-env NODE_ENV=production rollup -c -o dist/ng-redux.min.js", | ||
| "build": "$npm_execpath run build:commonjs && $npm_execpath run build:es && $npm_execpath run build:umd && $npm_execpath run build:umd:min", |
There was a problem hiding this comment.
The $npm_execpath fails on windows. I looked up the issue and it looks like the variable is not a standard. I couldn't find anything about it on NPM docs and the few places I checked out on Github that tried to use it either rolled back to npm or used some other method for checking the variable and falling back on npm.
There was a problem hiding this comment.
Are you basing this in the random guy from SO? I tested it on windows /shrug.
I'm making the change just to get it merged asap but I kinda disagree.
There was a problem hiding this comment.
No, no. I tested it and the variable does not exist at all in my environment and the command fails when I run it. I looked into a couple of repos that tried to use it and noticed that even repos that heavily support yarn, still reference NPM.
Here's the weird thing: I do have Yarn installed. So it seems that neither NPM nor Yarn add this variable by default. I did a clean install of NPM when NPM reached version 5.
There was a problem hiding this comment.
I want to mention that I'd rather we have this kind of compatibility for our yarn and NPM users than not but it seems like depending on that variable is not reliable :(
There was a problem hiding this comment.
Thats interesting, but as you said, since its not actually well documented, lets just keep npm until there is a supported way of doing it, my bad.
package.json
Outdated
| "build:umd": "cross-env NODE_ENV=development rollup -c -o dist/ng-redux.js", | ||
| "build:umd:min": "cross-env NODE_ENV=production rollup -c -o dist/ng-redux.min.js", | ||
| "build": "npm run build:commonjs && npm run build:es && npm run build:umd && npm run build:umd:min", | ||
| "test": "NODE_ENV=test mocha --compilers js:babel-register --recursive", |
There was a problem hiding this comment.
I hate to do this. I tried to push up my own change but since this is coming from your fork, I can't make changes. Can you add cross-env in front of the NODE_ENV?
Also, I sent you a collab invite :)
Updates all the dependencies and changes the build from webpack to
rollup exposing now multiple formats: