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

ESM build and refactoring #134

Merged
merged 3 commits into from
Jan 6, 2023
Merged

ESM build and refactoring #134

merged 3 commits into from
Jan 6, 2023

Conversation

edemaine
Copy link
Collaborator

@edemaine edemaine commented Jan 6, 2023

  • Add support for ESM .coffee files in coffee-esm.mjs (autodetection)
  • Build new ESM version dist/main.mjs, which Vite seems to need in New Playground tag in docs #125 (I've tested this in code I haven't yet pushed). (If I just told esbuild to convert the CJS code to ESM, you wouldn't be able to do import {compile} from '@danielx/civet'; only the default export gets preserved.)
  • CoffeeScript code refactored to use ESM (but some things like Hera parser are still CJS)
  • Note that default exports no longer have everything; need to import *

@edemaine
Copy link
Collaborator Author

edemaine commented Jan 6, 2023

Oh, in package.json, I'm not sure whether "exports"/"." should point to the .js or .mjs file... Vite was unhappy with the former, but it was fine when I explicitly imported ".../main.mjs".

Perhaps we should we be using "import" and "require" conditions?

@STRd6
Copy link
Contributor

STRd6 commented Jan 6, 2023

I think using import and require conditions is a good idea. My main goal is to have Civet "just work" whatever type of project people npm install into. And that probably means adding both ESM and CJS builds to the default dist with the proper package.json config.

@STRd6 STRd6 merged commit 5b29bac into master Jan 6, 2023
@edemaine edemaine deleted the esm branch January 8, 2023 17:55
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