Skip to content

Conversation

@henrist
Copy link
Contributor

@henrist henrist commented Jan 14, 2025

Description

Closes #1848

Motivation and Context

See #1848

How Has This Been Tested?

Tested on a service locally that failed previously. See #1848 for environment details.

Screenshots (if appropriate):

N/A

@henrist henrist changed the title Load in ESM mode if package.json type is module Load TypeScript in ESM mode if package.json type is module Jan 14, 2025
@DorianMazur
Copy link
Collaborator

DorianMazur commented Jan 17, 2025

@henrist Thank you for the PR! Can you create new test for this?

@henrist
Copy link
Contributor Author

henrist commented Jan 20, 2025

@DorianMazur Test added (and added a missing inclusion of the commonjs variant).

Making the test fail before my fixes requires a bit of code. For context this is the error:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /Users/henrste/repo/node_modules/dependency/package.json
    at exportsNotFound (node:internal/modules/esm/resolve:314:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:605:13)
    at resolveExports (node:internal/modules/cjs/loader:638:36)
    at Function._findPath (node:internal/modules/cjs/loader:743:31)
    at node:internal/modules/cjs/loader:1230:27
    at nextResolveSimple (/Users/henrste/repo/node_modules/.pnpm/tsx@4.19.2/node_modules/tsx/dist/register-DCnOAxY2.cjs:3:942)
    at /Users/henrste/repo/node_modules/.pnpm/tsx@4.19.2/node_modules/tsx/dist/register-DCnOAxY2.cjs:2:2550
    at /Users/henrste/repo/node_modules/.pnpm/tsx@4.19.2/node_modules/tsx/dist/register-DCnOAxY2.cjs:2:1624
    at resolveTsPaths (/Users/henrste/repo/node_modules/.pnpm/tsx@4.19.2/node_modules/tsx/dist/register-DCnOAxY2.cjs:3:760)
    at Function._resolveFilename (/Users/henrste/repo/node_modules/.pnpm/tsx@4.19.2/node_modules/tsx/dist/register-DCnOAxY2.cjs:3:1038)

When a package declares conditional exports using the exports field, the CommonJS loader does not load an import export.

@henrist
Copy link
Contributor Author

henrist commented Jan 27, 2025

@DorianMazur I think the failed tests are flakes in the repo (I see others failing too). Can you confirm?

@dherault dherault merged commit 8fb6397 into dherault:master Jan 31, 2025
6 of 9 checks passed
@dherault
Copy link
Owner

Thanks! It should be included in the next version.

@dherault
Copy link
Owner

@DorianMazur Could you run me through the process of creating a release please?

@henrist henrist deleted the fix-esm branch February 2, 2025 18:50
@DorianMazur
Copy link
Collaborator

@dherault Just run the Release workflow by creating a tag and then creating a release with it.

If it's v13.x.x, the npm tag will be legacy.
Otherwise, the tag will be latest.
The release process is basically the same, the only automated part is deploying to npm after creating the release on GitHub.

@VialFlorian
Copy link

I see this PR has been merged - that's great!
But it seems like it hasn't been released yet, any idea when this fix will be included in a new release?

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.

ESM mode not supported for TypeScript

4 participants