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

ensure esm, cjs, typescript builds are usable #122

Closed
1 of 5 tasks
michaelglass opened this issue Jul 4, 2024 · 3 comments
Closed
1 of 5 tasks

ensure esm, cjs, typescript builds are usable #122

michaelglass opened this issue Jul 4, 2024 · 3 comments

Comments

@michaelglass
Copy link
Collaborator

michaelglass commented Jul 4, 2024

I built a test repo with some test commands that should verify if we're packaging nlx libs correctly.

I don't think we are.

Some issues:

  • package.json expects the main field to be a commonjs lib (fixed in unify rollup #119)

  • (not a umd / web problem) node checks either the file extension (.mjs -> esm, .cjs -> commonjs) or the type: field in package.json to figure out whether the dependency should use common js or esm packaging. I thought this blog post was helpful
    Some solutions:

    1. we rename the two files .mjs / .cjs
      - pros: it works without a type field in package.json
      - cons: apparently it's not supported by all build tools

    2. we build two different package.jsons for js and esm
      - pros: it should work for all builds tools
      - cons: it's a bit more complicated

  • mixing default & named exports isn't properly supported by commonjs (or by our umd build)

let's resolve the second and third issue and

  • add the test-repo build scripts to CI
  • automate the browser packaging test and add it to CI as well (puppeteer?)
@michaelglass michaelglass changed the title ensure esm, cjs, typescript builds as expected ensure esm, cjs, typescript builds are usable Jul 4, 2024
@michaelglass
Copy link
Collaborator Author

@jakub-nlx asks in slack, "Do we need to support cjs?"

@peterszerzo
Copy link
Collaborator

peterszerzo commented Jul 5, 2024

@jakub-nlx @michaelglass we have one user left on a legacy webpack build system, we expect them to be off within the next few months. After that, I would say that we should 'support cjs in a scrappy way', as in I would be comfortable telling users that if they really want cjs, they need to change the import path to @nlxai/chat-core/lib/index.cjs if they want to get to the compatible file.

@michaelglass
Copy link
Collaborator Author

Ok then I propose a third option:

  1. we rename the common js file to .cjs
  2. we add a type:module to all the package.json files

let's see if that makes the test repo happy.

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

No branches or pull requests

2 participants