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

Undo (soft) breaking change introduced in 93c0a35 & #816. #930

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

indexzero
Copy link
Contributor

@indexzero indexzero commented Apr 20, 2017

We say "soft" breaking because it was not a strict contract between react-intl and
consumers that raw src/* files be included in the npm package. If one were to want
a custom build of react-intl for their own purposes the npm package is now no longer
a viable location for that source code.

We say "soft" breaking because it was not a strict contract between `react-intl` and
consumers that raw `src/*` files be included in the npm package. If one were to want
a custom build of `react-intl` for their own purposes the npm package is now no longer
a viable location for that source code.
@yahoocla
Copy link

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄

@Swaagie
Copy link

Swaagie commented Apr 21, 2017

@indexzero https://github.com/yahoo/react-intl/blob/master/package.json#L34 is exposed as ES6 from the npm package though. So the issue might be as simple as getting the builder to pickup on that over main

@indexzero
Copy link
Contributor Author

There is more complexity here. The logic behind a custom build generally centers around avoiding additional calls to addLocaleData. Even though there is an ES6 main now, that is a rollup based build, so there is no way to ignore src/en.js if you do not wish to include the default locale data.

Overall for the react-intl maintainers who will eventually read this: removing the source files from an npm package is a breaking change because the implicit contract you were making before was "hey, here's the raw individual source files if you want to use them directly". That contract was broken in #816. Overall those src files are only an additional 12k in your npm package:

tar -czvf src.tgz src/
ls -al src.tgz
-rw-r--r--  1 [redacted]  11673 Apr 21 13:35 src.tgz

... so while I can appreciate the desire for a smaller build this change has very little impact on anyone w.r.t. number of bytes downloaded.

@okuryu
Copy link
Member

okuryu commented Sep 1, 2017

Okay, let's merge this.

@okuryu okuryu merged commit fb2bc76 into formatjs:master Sep 1, 2017
@okuryu okuryu mentioned this pull request Sep 5, 2017
@indexzero indexzero deleted the no-npmignore-src branch December 12, 2017 11:30
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.

None yet

4 participants