Skip to content
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

fix(*): packages are being bundled twice #630

Merged
merged 1 commit into from
Dec 6, 2017

Conversation

alan-agius4
Copy link
Contributor

Remove JavaScript from src folder as this is not required by the consumers with Angular 4.x.x, @ngtools/webpack and paths in tsconfig.json it causes the src to be bundled togather with the FESM.

Closes: #629

Remove `JavaScript` from `src` folder as this is not required by the consumers with `Angular 4.x.x`, `@ngtools/webpack` and `paths` in `tsconfig.json` it causes the `src` to be bundled togather with the `FESM`.

Closes: ngrx#629
@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.274% when pulling 5aec884 on alan-agius4:feature/fix-double-bundle into 2fbcd24 on ngrx:master.

@brandonroberts
Copy link
Member

I'm still not inclined to merge this as seems like a configuration issue with the bundler. I've used NGRX V4 with Angular 4 and webpack wth no issues. Maybe @TheLarkInn can weigh in?

@alan-agius4
Copy link
Contributor Author

The problem is when you use NGRX 4, Angular 4 and Paths options in TSConfig.

NGC 4 reaolved the paths of the source files next to the .d.ts files instead of using the package.json if paths options is specified in the ts config.

This is no longer a problem in NG5 because Angular change how files are resolved, as they now rely on TypeScript to do the resolution.

It’s not the bundler that is resolving the files incorrectly but rather NGC, that said you are still shipping js files which in theory shouldn’t be present in the distributable as consumers shouldn’t be using, so why not delete them really? Will even make the final package smaller when downloading in npm.

So really this PR, fixes a bug, removes uneeded files from the distributable, and align your final package format with Google’s ‘Angular Package Format V 4.0’.

@brandonroberts
Copy link
Member

Good points, but I'm not going to introduce what could be a breaking change for many existing users to handle this edge case. We'll look at changing this in the next major release.

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Dec 6, 2017

Can you explain why this is a breaking change? Bundlers are still unable to pick up these sources because they are not listed in the package.json

@brandonroberts
Copy link
Member

It could be a breaking change, mainly for consumers who are reaching into the source files but you shouldn't be doing that anyway. If it affects someone using the nightlies, they will chime in. Thanks for your patience.

@brandonroberts brandonroberts merged commit b7a554a into ngrx:master Dec 6, 2017
@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Dec 6, 2017 via email

@alan-agius4 alan-agius4 deleted the feature/fix-double-bundle branch December 6, 2017 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants