-
Notifications
You must be signed in to change notification settings - Fork 44
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 native Node.js ES Modules support #74
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 tasks
This would be great! Thank you @abdonrd for putting in the work on the PR. |
I would love to see this merged in! |
benjamn
added a commit
to apollographql/apollo-client
that referenced
this pull request
Jan 25, 2021
Apollo Client currently uses the Observable implementation provided by the zen-observable npm package, since it is small and works well for our needs. Although zen-observable itself is not written in TypeScript, we use the companion package @types/zen-observable to provide types. We are actively considering switching from zen-observable to RxJS (#5749), though it remains to be seen how much of a breaking change that will be (and thus whether it makes sense for a minor or major version update). In the meantime, several issues (most recently #5961) have been opened pointing out that zen-observable effectively does not support importing its code as ECMAScript modules, even though it has a separate entry point (zen-observable/esm.js) that uses ESM syntax. This is a problem the zen-observable package could easily fix by adding a "module" field to its package.json, but @abdonrd tried to propose that change in a PR (as I requested), and has not heard back since June 2020: zenparsing/zen-observable#74 Fortunately, Apollo maintains an npm package called zen-observable-ts, which was originally created to provide TypeScript types, before @types/zen-observable was introduced. This commit revives that wrapper package, so we can make sure both CommonJS and ESM exports are supported, with full TypeScript types, until the zen-observable maintainer gets around to merging that PR. I considered forking zen-observable, but I'm happy with this wrapping strategy, since reusing zen-observable makes it easier to take advantage of any future updates to the zen-observable package. I also considered using https://www.npmjs.com/package/patch-package to apply changes to node_modules/zen-observable/package.json upon installation, but that doesn't really work unless @apollo/client bundles the zen-observable implementation into itself, since otherwise the original (unpatched) zen-observable package will continue to be installed when developers npm install @apollo/client. The patch-package tool is great, but I don't think it's meant for libraries to use. Thankfully, this time around we do not need to hand-write the TypeScript types, since they can be re-exported from @types/zen-observable. I bumped the major version of zen-observable-ts, since the older versions (used by apollo-link) still get more than two million downloads per week. The source code for the zen-observable-ts package can now be found at https://github.com/apollographql/zen-observable-ts, rather than the old https://github.com/apollographql/apollo-link monorepo.
benjamn
added a commit
to apollographql/apollo-client
that referenced
this pull request
Jan 25, 2021
Apollo Client currently uses the Observable implementation provided by the zen-observable npm package, since it is small and works well for our needs. Although zen-observable itself is not written in TypeScript, we use the companion package @types/zen-observable to provide types. We are actively considering switching from zen-observable to RxJS (#5749), though it remains to be seen how much of a breaking change that will be (and thus whether it makes sense for a minor or major version update). In the meantime, several issues (most recently #5961) have been opened pointing out that zen-observable effectively does not support importing its code as ECMAScript modules, even though it has a separate entry point (zen-observable/esm.js) that uses ESM syntax. This is a problem the zen-observable package could easily fix by adding a "module" field to its package.json, but @abdonrd tried to propose that change in a PR (as I requested), and has not heard back since June 2020: zenparsing/zen-observable#74 Fortunately, Apollo maintains an npm package called zen-observable-ts, which was originally created to provide TypeScript types, before @types/zen-observable was introduced. This commit revives that wrapper package, so we can make sure both CommonJS and ESM exports are supported, with full TypeScript types, until the zen-observable maintainer gets around to merging that PR. I considered forking zen-observable, but I'm happy with this wrapping strategy, since reusing zen-observable makes it easier to take advantage of any future updates to the zen-observable package. I also considered using https://www.npmjs.com/package/patch-package to apply changes to node_modules/zen-observable/package.json upon installation, but that doesn't really work unless @apollo/client bundles the zen-observable implementation into itself, since otherwise the original (unpatched) zen-observable package will continue to be installed when developers npm install @apollo/client. The patch-package tool is great, but I don't think it's meant for libraries to use. Thankfully, this time around we do not need to hand-write the TypeScript types, since they can be re-exported from @types/zen-observable. I bumped the major version of zen-observable-ts, since the older versions (used by apollo-link) still get more than two million downloads per week. The source code for the zen-observable-ts package can now be found at https://github.com/apollographql/zen-observable-ts, rather than the old https://github.com/apollographql/apollo-link monorepo.
Closing because no response. |
sharing comments here from a similar thread for those following along #69 (comment) 👀 |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR I add native Node.js ES Modules support.
Following the same recommendations as the Rollup plugins:
Fix #62.
Also fix apollographql/apollo-client#5961.