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

Can't use ESM packages when linking a CJS plugin into a CJS CLI #757

Closed
cristiand391 opened this issue Aug 4, 2023 · 4 comments · Fixed by #759
Closed

Can't use ESM packages when linking a CJS plugin into a CJS CLI #757

cristiand391 opened this issue Aug 4, 2023 · 4 comments · Fixed by #759
Labels
bug Something isn't working ESM Related to ESM support

Comments

@cristiand391
Copy link
Member

cristiand391 commented Aug 4, 2023

Describe the bug
I can't use an ESM package in a CJS plugin if I link it into a CJS CLI.

Node supports loading an ESM package in commonjs code via await import and this works when running commands with bin/run and bin/dev if you set moduleResolution to node16 in the tsconfig but not when linking the plugin.
This only affects linked plugins, it will work if you publish and install the plugin.

To Reproduce

  1. Clone https://github.com/cristiand391/hello-world
  2. yarn install && yarn build
  3. ./bin/run hello world works, same with bin/dev
  4. link it: sf plugins link .
  5. run sf hello world, you get:
➜  hello-world git:(main) sf hello world
    Error: require() of ES Module /Users/cdominguez/code/gh/hello-world/node_modules/chalk/source/index.js from
    /Users/cdominguez/code/gh/hello-world/src/commands/hello/world.ts not supported.
    Instead change the require of index.js in /Users/cdominguez/code/gh/hello-world/src/commands/hello/world.ts to a dynamic import() which is available in all CommonJS
     modules.
    Code: ERR_REQUIRE_ESM

Expected behavior
linked command should work fine.

Findings
I compiled the command with ts-node to verify that it was respecting my tsconfig and not replacing the await import, looks correct here:
Screenshot 2023-08-04 at 16 02 30

then looked at the ts-node code in oclif/core and found that it doesn't pass the moduleResolution prop here:
https://github.com/oclif/core/blob/main/src/config/ts-node.ts#L59-L72

I tested this by modifying the oclif/core/lib/config/ts-node.js file to conditionally set it and worked but I'm not sure how it could affect CJS<->ESM interop.

In the lower right corner you can see there's 2 linked plugins, but only hello-world has moduleResolution set to node16. The conditional approach doesn't seem to break plugins without this prop set.
Screenshot 2023-08-04 at 16 20 37

@mdonnalley mdonnalley added the ESM Related to ESM support label Aug 4, 2023
@github-project-automation github-project-automation bot moved this to Backlog in ESM Support Aug 4, 2023
@mdonnalley mdonnalley added the bug Something isn't working label Aug 9, 2023
@git2gus
Copy link

git2gus bot commented Aug 9, 2023

This issue has been linked to a new work item: W-13915423

@mdonnalley
Copy link
Contributor

Possibly related to oclif/oclif#695

@mdonnalley mdonnalley mentioned this issue Aug 14, 2023
24 tasks
@mdonnalley mdonnalley moved this from Backlog to Waiting in ESM Support Aug 14, 2023
@mdonnalley
Copy link
Contributor

This will be fixed in #759 provided that both the root plugin and linked plugin have `moduleResolution: node16" in their tsconfigs

@mdonnalley
Copy link
Contributor

Fixed by #759 (currently available in @ocilf/[email protected])

@github-project-automation github-project-automation bot moved this from Waiting to Closed in ESM Support Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ESM Related to ESM support
Projects
No open projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

2 participants