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.

Sign up for GitHub

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

Jump to bottom

Convert project to esm #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Convert project to esm #24

wants to merge 2 commits into from

Conversation

TrySound
Copy link
Contributor

In this diff I migrated the project to esm and provided module field
which is useful for bundlers.

My usecase is rollup. It is able to handle only esm. And this package
requires me to add commonjs and json plugins.

As a side bonus this module can be used in browsers without bundlers
similar to this https://unpkg.com/jss@next?module.

For tap I wasn't able to setup esm so I replaced it with tape.

@TrySound
Copy link
Contributor Author

Hi @fannarsh! What do you think about this change?

@fannarsh
Copy link
Owner

Hi @TrySound
I'm quite busy at the moment, but I should be able to take a look at this after this weekend.

@TrySound
Copy link
Contributor Author

Friendly ping @fannarsh if you have some time :)

@TrySound
Copy link
Contributor Author

TrySound commented Jul 5, 2019

Hi again, if you want I can split migrating to tape in a separate PR for simpler review.

@fannarsh
Copy link
Owner

fannarsh commented Jul 5, 2019

Hi,
sorry for the late comment, I got side tracked the other day and forgot to come back to this.
The PR itself is fine, thanks for that.
But I have decided that I'm not going to convert the project to esm.
Although I will look into providing a esm build for it.

(I'm not going to close the PR right away, I might change my mind.)

@TrySound
Copy link
Contributor Author

TrySound commented Jul 5, 2019

Let me know if you need to clarify something.

@TrySound TrySound mentioned this pull request Jul 5, 2019
Merged
@TrySound
Convert project to esm
dc21ad9 
In this diff I migrated the project to esm and provided module field
which is useful for bundlers.

My usecase is rollup. It is able to handle only esm. And this package
requires me to add commonjs and json plugins.

As a side bonus this module can be used in browsers without bundlers
similar to this https://unpkg.com/jss@next?module.

For `tap` I wasn't able to setup esm so I replaced it with `tape`.
@TrySound
Copy link
Contributor Author

I just found that tap supports esm out of the box. I reverted tape back to tap.

@sebinsua
Copy link

sebinsua commented Jan 30, 2020 •
edited
Loading

Any reason this hasn't been merged?

I just searched around and I haven't been able to find an ISO 3166-1-alpha-2 country code package that provides ES modules.

I do think there's an opportunity to provide data like country and language ISO codes but avoid bloating bundle sizes.

@Timebutt
Copy link

@fannarsh: you mentioned you'd take a look at providing an esm build? In the upcoming Angular 10 there is now a warning against including CommonJS packages in your dependencies so I'm guessing this issue will get more tracking soon.

Warning message as found in the output of the Angular 10 build:

WARNING in /file.ts depends on country-list. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://web.dev/commonjs-larger-bundles

@VictorRibeiroKCX
Copy link

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@TrySound @fannarsh @sebinsua @Timebutt @VictorRibeiroKCX