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

Adding ES Modules to the final build #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dkzlv
Copy link

@dkzlv dkzlv commented Apr 25, 2021

Currently, if you have a ES bundler (Vite, Snowpack) you cannot use this library, as it uses an outdated module system (UMD).
I added the support for ES modules. It just builds 4 additional files for both versions of the lib.
Most bundlers (Vite including) will use index.module.js by default, but it's a convention to also add a module key in the package.json.

@dkzlv
Copy link
Author

dkzlv commented Apr 25, 2021

If new versions are planned, I would actually advise in favor of abandoning rollup config at all and using something like dual-publish with CJS as a more common option for module system.

It would also drastically simplify the build process for you.

@g-harel
Copy link
Owner

g-harel commented May 19, 2021

Hey, so sorry I missed this. Will take a look ASAP

@g-harel
Copy link
Owner

g-harel commented May 19, 2021

Do you think publishing the TypeScript source could also solve this for you? I'm mostly outputting bundled JS to make it easier to use with a <script> import.

The build command fails because of uglify when I patch that rollup config change, I'll need to iterate on it.

@dkzlv
Copy link
Author

dkzlv commented May 19, 2021

@g-harel

I'm mostly outputting bundled JS to make it easier to use with a <script> import.

Most of the libs ship bundled code, that is ok. Not only it allows to use stuff, like unpkg and other CDNs right in the script tag, it also speeds up the build, as I don't need to rebuild your library from the source.

Do you think publishing the TypeScript source could also solve this for you?

I believe it's not a common thing to ship raw .ts files. I didn't find a single example of this in my node_modules, tbh. I think it would work, but it's unnecessary work for the compiler and bundler.

As of #4, I don't see any errors, when I try to import the same file. TS doesn't throw any errors and compiles the code successfully. I guess there might be some incompatible configuration settings in tsconfig.json.

@g-harel
Copy link
Owner

g-harel commented May 27, 2021

I just published 2.2.1-beta.0 with this change, let me know how that works for you! If it works out I'll merge and do a real release

g-harel added a commit that referenced this pull request May 27, 2021
@dkzlv
Copy link
Author

dkzlv commented Jun 13, 2021

@g-harel I tried your beta release, but it doesn't work. I suspect it's mostly because you haven't added "module" key in package.json.
All these modern bundlers are so fragile :)

g-harel added a commit that referenced this pull request Jun 15, 2021
@g-harel
Copy link
Owner

g-harel commented Jun 15, 2021

That's embarrassing, my bad and thanks for checking. I published a patched version as 2.2.1-beta.1

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