-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add es2015 entries to the exports declaration #6614
Conversation
@IgorMinar @crisbeto .. Is there something we should document, or add to our documentation about how users can leverage this? This seems non-standard. Looking at the documentation for the exports field should this be |
The condition is indeed non-standard. The thought process here was that using the However, if it is acceptable for an import statement/expression to bring in ES2015 code with rxjs 6/7 then the |
Based on ReactiveX#6613 (comment). Adds `es2015` entries to the `exports` declaration in the `package.json`.
fdcafa2
to
939b81d
Compare
RxJS is still used pretty widely in Node. I'm not sure we can remove the CJS version yet. |
All LTS versions of node support ESM now. The main consideration for keeping CJS a bit longer is other packages that:
I suspect that this will extremely rare for any libraries that want to upgrade to RxJS@8 assuming that that's 6+ months away. |
CJS is not deprecated format while all of LTS node.js started support ESM. Also wide number of tooling ecosystem are not ready for pure esm packages (TypeStrong/ts-node#1007, electron/electron#21457, jestjs/jest#9430 for some of examples). I do not think at this rate when we release 8 majority of users are ready to switch from CJS to ESM. Deprecating CJS in RxJS and supporting tree-shakable ESM (es2015) and native ESM is different story. We can discuss to support better ESM in the future version of RxJS but CJS may need live bit longer. |
@kwonoj sgtm, the removal of CJS is optional. The only downside of keeping it longer than necessary is the build/test/release complexity and package size. Let's revisit CJS in the future, when v8 is almost fully baked. |
@benlesh unlike with PR #6613, we do believe that the issue addressed in this PR needs to be addressed with a bit of urgency because without this change, we can't recommend RxJS v7 to be the default in Angular CLI applications. Doing so would without this issue fixed, Angular apps (and in fact any web app using RxJS) would default to using es5 as the input into the bundler which results in major size regressions. In the ideal case What can we do to help resolve this issue and enable Angular ecosystem to use RxJS v7 by default rather than as an opt-in? Thank you. |
This PR fixes #6321 and unfortunately blocks adoption of RxJS v7 by the Angular ecosystem. We'd love for it to be merged and released soon because without this fix any (even non-Angular) application adopting RxJS v7 will see a payload size regression. Thank you for considering. |
Core team meeting: Agreement that we can merge this in, as it's non-breaking. @IgorMinar mentioned that the Angular team would be willing to document how non-Angular projects can leverage this non-standard config. Auxiliary discussion: As far as Angular is concerned the latest version of ECMAScript target the better (ES2020 is what they're targeting/using now) |
Based on #6613 (comment). Adds
es2015
entries to theexports
declaration in thepackage.json
.