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

chore: provide CJS and ESM bundle #234

Closed

Conversation

JoviDeCroock
Copy link

@JoviDeCroock JoviDeCroock commented Jan 17, 2019

This PR allows us to follow the mainstream way of publishing a library, giving us the ability to provide a tree-shakeable esm bundle, a cjs bundle for main and an umd bundle for browser.

As mentioned in: #229
And as done in other repositories from apollo:

I also think it's quite safe to say that it would be appropriate to upgrade the rollup and babel deps, if this is desired I feel up to the task. Just reply it here

@JoviDeCroock JoviDeCroock changed the title chore: provide another way of bundling chore: provide CJS and ESM bundle Jan 17, 2019
@benjamn
Copy link
Member

benjamn commented Jan 25, 2019

I think there's a more fundamental issue that should precede this work (though I fully support standardizing our publishing formats): this library is still just CommonJS rather than TypeScript or even ESM. Not asking you to do that conversion yourself, but I think it's time someone did it!

@JoviDeCroock
Copy link
Author

JoviDeCroock commented Jan 25, 2019

Allright, I feel up for that. So you would like to see this repository converted to TypeScript. I'll probably have some time to do that this sunday. :D thanks for looking at this already. I'l lleave this open while I do the conversion or?

Are the flow types obsolete then or? Don't have any flow experience

@JoviDeCroock
Copy link
Author

To be honest, I've been giving it a try but converting this repository to TypeScript is non-trivial. A lot of types that should be a 1-1 mapping from graphql/language are a bit misused it feels like... Could be just me misunderstanding some parts

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.

2 participants