angular-redux / ng-redux

Angular bindings for Redux
MIT License
1.16k stars 178 forks source link

Exclude Redux lib from builds #178

Closed kaidjohnson closed 6 years ago

kaidjohnson commented 6 years ago

I agree that the redux library should not be a baked-in dependency of the /dist builds of ng-redux. Simple enough to exclude using rollup globals.

Fixes #70

deini commented 6 years ago

Hey @kaidjohnson thanks for opening this PR! A couple of things:

  1. This is going to be a Breaking change, so please open this PR against the release/4.0.0 branch instead of master 🙏

  2. Since 4.0.0 wont have a /dist directory lets remove those changes for now

  3. Lets move redux from dependencies to devDependencies in package.json and then add redux as a peer dependency too:

    "peerDependencies": {
    "redux": "^3.0.0"
    },
  4. Instead of having it as globals let make it part of externals you can checkout rollups guide on peer Dependencies.

  5. You will need to add the externals to the umd build, however since we moved the dependency to peerDependencies it will break es/cjs builds, so you would have to modify that external to accept what it currently does + redux.

  6. We are going to start enforcing conventional commits (#180, #181) so please change your commit to follow the standard. Eg: something like:

    
    chore(build): Make redux a peerDependency

BREAKING CHANGE: Redux is not going to be part of the umd bundle anymore



Let me know if you have any questions or need help with any of the points stated above.
kaidjohnson commented 6 years ago

Looks like release/4.0.0 already includes redux as an external, so the issue should be resolved already on that branch. Fantastic! And thanks for the quick response :)