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

Implement ES6 export file #325

Merged
merged 4 commits into from
Jan 29, 2021
Merged

Implement ES6 export file #325

merged 4 commits into from
Jan 29, 2021

Conversation

PowerKiKi
Copy link
Contributor

@PowerKiKi PowerKiKi commented Nov 6, 2020

This upgrade all dev dependencies in order to implement ES6 exports
in order to solve #303.

This also remove yarn.lock entirely because this package is meant to
work with npm only as can be seen from the mention of npm command in
package.json.

Fixes #303
Closes #272
Closes #273

@PowerKiKi
Copy link
Contributor Author

This PR is mostly a rebase of #273

@matthewerwin
Copy link
Contributor

What's the hangup on getting this merged?

@PowerKiKi
Copy link
Contributor Author

@benjamn may I asked you to either review yourself or assign a reviewer please ?

Both this PR and #273 should be relatively quick to review as there is no logic changes in the package itself. It's mostly configuring the tools properly. And I believe it would be a welcome change for the community.

@benjamn benjamn requested review from hwillson and benjamn January 19, 2021 16:46
@benjamn
Copy link
Member

benjamn commented Jan 19, 2021

@PowerKiKi Thanks for the ping! I recently got my GitHub notifications under control(-ish), so hopefully I won't miss stuff like this in the future.

@hwillson Any thoughts/recollections as to why we hadn't done this before? Was it just that we wanted to convert the package to TypeScript first?

@hwillson
Copy link
Member

Was it just that we wanted to convert the package to TypeScript first?

Exactly!

@benjamn
Copy link
Member

benjamn commented Jan 25, 2021

Another ESM-incompatible npm package used by @apollo/client that we're updating/wrapping to be more ESM-friendly: apollographql/apollo-client#7615

@benjamn benjamn self-assigned this Jan 29, 2021
PowerKiKi and others added 2 commits January 29, 2021 17:15
This upgrade all dev dependencies in order to implement ES6 exports
in order to solve apollographql#303.

This also remove yarn.lock entirely because this package is meant to
work with npm only as can be seen from the mention of npm command in
package.json.

Fixes apollographql#303
Closes apollographql#272
Closes apollographql#273
The loader.js module is still CommonJS, since that's what webpack wants,
and importing the package with require("graphql-tag") still returns the
gql function, rather than an exports object, for backwards compatibility.

I converted the tests to TypeScript, since the type checking is useful for
test code. Mocha now runs a CommonJS test bundle generated by Rollup.
@benjamn benjamn changed the base branch from master to main January 29, 2021 22:15
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting this started @PowerKiKi, and please let us know if you have any concerns about the additional commits that I pushed.

@benjamn benjamn merged commit a9e1cc9 into apollographql:main Jan 29, 2021
benjamn added a commit to apollographql/apollo-client that referenced this pull request Jan 29, 2021
@PowerKiKi
Copy link
Contributor Author

Thank you for merging this.

I would like to emphasize that all credits should go to @abdonrd 🎉 . I merely rebased his work.

@abdonrd
Copy link
Contributor

abdonrd commented Feb 1, 2021

Thanks for the mention @PowerKiKi!

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.

Implement ES6 export file
5 participants