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

fix: issue where esm did not build with fully specified extensions #171

Closed
wants to merge 6 commits into from

Conversation

jessecollier
Copy link
Contributor

Summary & Motivation

Previously using a tsconfig.esm.json would compile output for esm, however paths were relative with no extensions.

Typically this is solved by using fully specified extensions in imports in TS files. However, this will not work in a dual output module (cjs and esm).

To resolve this, rollup is used to compile and export both cjs and esm as well as types.

Additionally, by default rollup will bundle everything into one file. To preserve file structure, this is disabled.

Additionally, without specifiying type: module in the package json all ES modules need the mjs extension to be interpreted by downstream compilers as ES modules.

How I Tested These Changes

Did you add a changeset?

To add a changeset for your pr run pnpm changeset. pnpm changest will generate a file where you should write a human friendly message about the changes. Note how this (example) includes the package name (should be auto added by the command) along with the type of semver change (major.minor.patch) (which you should set).

These changes will be used at release time to determine what packages to publish and how to bump their version. For more context see this comment.

Copy link

codesandbox-ci bot commented Nov 24, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
rollup 4.5.2 eval, filesystem, environment +0 2.16 MB lukastaegert

Copy link
Contributor

@r-n-o r-n-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of things to resolve before merging:

  • The js-build step is failing (looks like relative imports don't work anymore? Seems strange)
  • We need a changeset entry for this to make sure we properly release packages that are affected. Should be a minor bump for all I believe? pnpm run changeset should guide you!

@r-n-o r-n-o mentioned this pull request Nov 27, 2023
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