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: esm module resolve issues #754

Merged
merged 4 commits into from
Mar 4, 2024
Merged

fix: esm module resolve issues #754

merged 4 commits into from
Mar 4, 2024

Conversation

yeliex
Copy link
Contributor

@yeliex yeliex commented Feb 5, 2024

this commit fix some esm module resolve bugs

  1. convert fileUrl to path before compile in esm, which may cause path alias resolve issue to align with previous behavior. close @swc-node/register@1.8.0 /esm-register does not recognise tsconfig.json paths #753
  2. remove baseUrl from esm to keep module import specifier, as we use tsc resolver.

this commit convert fileUrl to path before compile in esm, this may cause path alias resolve issue

close swc-project#753
Copy link

changeset-bot bot commented Feb 5, 2024

🦋 Changeset detected

Latest commit: bd4e32f

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@yeliex yeliex changed the title fix: convert fileUrl to path before compile WIP: fix: convert fileUrl to path before compile Feb 5, 2024
@psychobolt
Copy link

@yeliex Hi I have a similar issue, but when the resolve's specifier begins with file protocol e.g. file://. I current have this patch:

diff --git a/esm/esm.mjs b/esm/esm.mjs
index 8453ae2480391c8eb5cc18d5460bd55b723592ce..a62fa5753df9a00f9680adc8dea7b005dea82f6e 100644
--- a/esm/esm.mjs
+++ b/esm/esm.mjs
@@ -13,7 +13,8 @@ const host = {
 };
 const EXTENSIONS = [ts.Extension.Ts, ts.Extension.Tsx, ts.Extension.Mts];
 export const resolve = async (specifier, context, nextResolve) => {
-    const isTS = EXTENSIONS.some((ext) => specifier.endsWith(ext));
+    const modulePath = specifier.startsWith('file://') ? fileURLToPath(specifier) : specifier;
+    const isTS = EXTENSIONS.some((ext) => modulePath.endsWith(ext));
     // entrypoint
     if (!context.parentURL) {
         return {
@@ -26,7 +27,7 @@ export const resolve = async (specifier, context, nextResolve) => {
     if (context.parentURL.includes('/node_modules/') && !isTS) {
         return nextResolve(specifier);
     }
-    const { resolvedModule } = ts.resolveModuleName(specifier, fileURLToPath(context.parentURL), tsconfig, host, moduleResolutionCache);
+    const { resolvedModule } = ts.resolveModuleName(modulePath, fileURLToPath(context.parentURL), tsconfig, host, moduleResolutionCache);
     // import from local project to local project TS file
     if (resolvedModule &&
         !resolvedModule.resolvedFileName.includes('/node_modules/') &&

@yeliex
Copy link
Contributor Author

yeliex commented Feb 18, 2024

@psychobolt This caused a ci failure, maybe a complication issue with @swc/core. I am trying to resolve this, and filling more test cases

@yeliex yeliex changed the title WIP: fix: convert fileUrl to path before compile fix: esm module resolve bugs Mar 3, 2024
@yeliex
Copy link
Contributor Author

yeliex commented Mar 3, 2024

The pr should fix these issues as well, but not test for each. feedback required.

#758
#753
#734
#727 (comment)

@yeliex yeliex changed the title fix: esm module resolve bugs fix: esm module resolve issues Mar 3, 2024
Copy link

@mylesj mylesj left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this @yeliex. I tried this against my original bug report and can confirm that this runs as expected with the newer --import loader. However it unfortunately also breaks the original register method.

Error: failed to handle: base_dir(``) must be absolute. Please ensure that jsc.baseUrl is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if .swcrc is used, but if not, there are many candidates. e.g. the directory containing package.json, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it.

Full stack trace
❯ pn esm

> swc-node-esm-register-issue@ esm /Users/myles/dev/issue-swc-node-1
> node --import @swc-node/register/esm-register src/index.ts

hello
❯ pn cjs

> swc-node-esm-register-issue@ cjs /Users/myles/dev/issue-swc-node-1
> node -r @swc-node/register src/index.ts

thread '<unnamed>' panicked at /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/swc_ecma_transforms_module-0.179.5/src/path.rs:122:13:
base_dir(``) must be absolute. Please ensure that `jsc.baseUrl` is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if `.swcrc` is used, but if not, there are many candidates. e.g. the directory containing `package.json`, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it. If you see this error, please report an issue to the package author.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc+core@1.3.107/node_modules/@swc/core/index.js:244
            return bindings.transformSync(isModule ? JSON.stringify(src) : src, isModule, toBuffer(newOptions));
                            ^

Error: failed to handle: base_dir(``) must be absolute. Please ensure that `jsc.baseUrl` is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if `.swcrc` is used, but if not, there are many candidates. e.g. the directory containing `package.json`, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it. If you see this error, please report an issue to the package author.
    at Compiler.transformSync (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc+core@1.3.107/node_modules/@swc/core/index.js:244:29)
    at transformSync (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc+core@1.3.107/node_modules/@swc/core/index.js:351:21)
    at transformSync (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc-node+core@1.12.0_@swc+core@1.3.107_@swc+types@0.1.5/node_modules/@swc-node/core/index.ts:90:26)
    at compile (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/file+.yalc+@swc-node+register_@swc+core@1.3.107_@swc+types@0.1.5_typescript@5.3.3/node_modules/@swc-node/register/register.ts:105:40)
    at exts (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/file+.yalc+@swc-node+register_@swc+core@1.3.107_@swc+types@0.1.5_typescript@5.3.3/node_modules/@swc-node/register/register.ts:116:38)
    at Module._compile (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/pirates@4.0.6/node_modules/pirates/lib/index.js:113:29)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
    at Object.newLoader [as .ts] (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/pirates@4.0.6/node_modules/pirates/lib/index.js:121:7)
    at Module.load (node:internal/modules/cjs/loader:1207:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1023:12) {
  code: 'GenericFailure'
}

Node.js v20.11.0
 ELIFECYCLE  Command failed with exit code 1.

@kdy1 kdy1 self-assigned this Mar 4, 2024
@kdy1 kdy1 merged commit d35ddf1 into swc-project:master Mar 4, 2024
7 checks passed
@cghiurea
Copy link

cghiurea commented Mar 4, 2024

The pr should fix these issues as well, but not test for each. feedback required.

#758 #753 #734 #727 (comment)

@yeliex I am willing to test this on our project. Any instructions on how I can install the module from this pr? Or is it already published ?

@yeliex
Copy link
Contributor Author

yeliex commented Mar 4, 2024

The pr should fix these issues as well, but not test for each. feedback required.

#758 #753 #734 #727 (comment)

@yeliex I am willing to test this on our project. Any instructions on how I can install the module from this pr? Or is it already published ?

install package as git url or link locally

@yeliex
Copy link
Contributor Author

yeliex commented Mar 4, 2024

Thanks for looking into this @yeliex. I tried this against my original bug report and can confirm that this runs as expected with the newer --import loader. However it unfortunately also breaks the original register method.

Error: failed to handle: base_dir(``) must be absolute. Please ensure that jsc.baseUrl is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if .swcrc is used, but if not, there are many candidates. e.g. the directory containing package.json, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it.

Full stack trace

❯ pn esm



> swc-node-esm-register-issue@ esm /Users/myles/dev/issue-swc-node-1

> node --import @swc-node/register/esm-register src/index.ts



hello

❯ pn cjs



> swc-node-esm-register-issue@ cjs /Users/myles/dev/issue-swc-node-1

> node -r @swc-node/register src/index.ts



thread '<unnamed>' panicked at /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/swc_ecma_transforms_module-0.179.5/src/path.rs:122:13:

base_dir(``) must be absolute. Please ensure that `jsc.baseUrl` is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if `.swcrc` is used, but if not, there are many candidates. e.g. the directory containing `package.json`, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it. If you see this error, please report an issue to the package author.

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc+core@1.3.107/node_modules/@swc/core/index.js:244

            return bindings.transformSync(isModule ? JSON.stringify(src) : src, isModule, toBuffer(newOptions));

                            ^



Error: failed to handle: base_dir(``) must be absolute. Please ensure that `jsc.baseUrl` is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if `.swcrc` is used, but if not, there are many candidates. e.g. the directory containing `package.json`, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it. If you see this error, please report an issue to the package author.

    at Compiler.transformSync (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc+core@1.3.107/node_modules/@swc/core/index.js:244:29)

    at transformSync (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc+core@1.3.107/node_modules/@swc/core/index.js:351:21)

    at transformSync (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/@swc-node+core@1.12.0_@swc+core@1.3.107_@swc+types@0.1.5/node_modules/@swc-node/core/index.ts:90:26)

    at compile (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/file+.yalc+@swc-node+register_@swc+core@1.3.107_@swc+types@0.1.5_typescript@5.3.3/node_modules/@swc-node/register/register.ts:105:40)

    at exts (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/file+.yalc+@swc-node+register_@swc+core@1.3.107_@swc+types@0.1.5_typescript@5.3.3/node_modules/@swc-node/register/register.ts:116:38)

    at Module._compile (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/pirates@4.0.6/node_modules/pirates/lib/index.js:113:29)

    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)

    at Object.newLoader [as .ts] (/Users/myles/dev/issue-swc-node-1/node_modules/.pnpm/pirates@4.0.6/node_modules/pirates/lib/index.js:121:7)

    at Module.load (node:internal/modules/cjs/loader:1207:32)

    at Function.Module._load (node:internal/modules/cjs/loader:1023:12) {

  code: 'GenericFailure'

}



Node.js v20.11.0

 ELIFECYCLE  Command failed with exit code 1.

it looks not the same issue. i would looking this later today

@yeliex yeliex deleted the patch-1 branch March 4, 2024 01:49
@yeliex
Copy link
Contributor Author

yeliex commented Mar 4, 2024

Thanks for looking into this @yeliex. I tried this against my original bug report and can confirm that this runs as expected with the newer --import loader. However it unfortunately also breaks the original register method.

Error: failed to handle: base_dir(``) must be absolute. Please ensure that jsc.baseUrl is specified correctly. This cannot be deduced by SWC itself because SWC is a transpiler and it does not try to resolve project details. In other words, SWC does not know which directory should be used as a base directory. It can be deduced if `.swcrc` is used, but if not, there are many candidates. e.g. the directory containing `package.json`, or the current working directory. Because of that, the caller (typically the developer of the JavaScript package) should specify it.

Full stack trace

@mylesj add "baseUrl": "./" to tsconfig should resolve commonjs register issue.It is better to add a format validation to readDefaultTsConfig to align with swc/cli (or we should default use path of tsconfig.json to align with tsc cause this config reads from tsconfig? https://www.typescriptlang.org/tsconfig#paths) @Brooooooklyn @kdy1

@mylesj
Copy link

mylesj commented Mar 4, 2024

Thanks that does indeed address the issue locally, I just mentioned it to highlight the breaking API change. TypeScript documents the following under the baseUrl config so I think it is in your interest to align. My understanding of swc and associated tooling is that it is intended as a drop-in replacement for tsc - or at least that has become an expectation.

As of TypeScript 4.1, baseUrl is no longer required to be set when using paths.

@yeliex
Copy link
Contributor Author

yeliex commented Mar 4, 2024

Thanks that does indeed address the issue locally, I just mentioned it to highlight the breaking API change. TypeScript documents the following under the baseUrl config so I think it is in your interest to align. My understanding of swc and associated tooling is that it is intended as a drop-in replacement for tsc - or at least that has become an expectation.

As of TypeScript 4.1, baseUrl is no longer required to be set when using paths.

I have created a new PR to fix this, keeping the behavior with tsc and the previous versions of swc-node.

I Agree that it is better for swc-node/register to be compatible with ts-node(Attention: swc is not exactly the same as tsc, and should not be).

@alfaproject
Copy link

So, there's still some issue with resolves. TS paths now work correctly which is awesome but now deep imports from aws-sdk package for example, break:

> node --import @swc-node/register/esm-register ./apps/lambda-api/src/extract-schema.ts && prettier --write ./apps/lambda-api/src/schema.graphql


node:internal/process/esm_loader:34
      internalBinding('errors').triggerUncaughtException(
                                ^
Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/joaodias/the-mill/lambda-gaming.nosync/node_modules/aws-sdk/clients/dynamodb' imported from /Users/joaodias/the-mill/lambda-gaming.nosync/libs/node-aws/src/dynamodb/dynamodb-sdk.ts
Did you mean to import aws-sdk@2.1536.0/node_modules/aws-sdk/clients/dynamodb.js?
    at finalizeResolution (node:internal/modules/esm/resolve:264:11)
    at moduleResolve (node:internal/modules/esm/resolve:917:10)
    at defaultResolve (node:internal/modules/esm/resolve:1130:11)
    at nextResolve (node:internal/modules/esm/hooks:865:28)
    at resolve (file:///Users/joaodias/the-mill/lambda-gaming.nosync/node_modules/.pnpm/@swc-node+register@1.9.0_@swc+core@1.4.2_@swc+types@0.1.5_typescript@5.2.2/node_modules/@swc-node/register/esm/esm.mjs:44:12)
    at nextResolve (node:internal/modules/esm/hooks:865:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:303:30)
    at handleMessage (node:internal/modules/esm/worker:196:24)
    at Immediate.checkForMessages [as _onImmediate] (node:internal/modules/esm/worker:138:28)
    at process.processImmediate (node:internal/timers:478:21) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file:///Users/joaodias/the-mill/lambda-gaming.nosync/node_modules/aws-sdk/clients/dynamodb'
}

@yeliex
Copy link
Contributor Author

yeliex commented Mar 7, 2024

@alfaproject could u please provide a minimal reproduction

@alfaproject
Copy link

@yeliex the problem seems to be lack of .js file extension in imports but it's a very big project so I think I'll continue using CJS for now ):

@yeliex
Copy link
Contributor Author

yeliex commented Mar 20, 2024

@yeliex the problem seems to be lack of .js file extension in imports but it's a very big project so I think I'll continue using CJS for now ):

maybe the same as packages/register/test/ts-compiler-options-to-swc-config.spec.ts

check your baseUrl and paths config in tsconfig and swcrc

@alfaproject
Copy link

I get the same error with or without baseUrl. This is a Nx mono repo so I also tried to run the command in the subfolder of the project but then I get another error related to paths. Oh well...

I really appreciate your help but I think I'll need to drop this for the time being. I couldn't get our Jest tests to work with full ESM either so either way, we are kind of blocked in having everything ESM first, unfortunately

tsang-bot bot referenced this pull request in tsangste/nx-package-test May 5, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@swc-node/register](https://github.com/swc-project/swc-node) |
[`1.9.0` ->
`1.9.1`](https://renovatebot.com/diffs/npm/@swc-node%2fregister/1.9.0/1.9.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@swc-node%2fregister/1.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@swc-node%2fregister/1.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@swc-node%2fregister/1.9.0/1.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@swc-node%2fregister/1.9.0/1.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>swc-project/swc-node (@&#8203;swc-node/register)</summary>

###
[`v1.9.1`](https://github.com/swc-project/swc-node/releases/tag/%40swc-node/register%401.9.1)

[Compare
Source](https://github.com/swc-project/swc-node/compare/@swc-node/register@1.9.0...@swc-node/register@1.9.1)

#### What's Changed

- fix: esm module resolve issues by
[@&#8203;yeliex](https://github.com/yeliex) in
[https://github.com/swc-project/swc-node/pull/754](https://github.com/swc-project/swc-node/pull/754)
- fix: add default tsconfig.baseUrl to align with tsc behavior by
[@&#8203;yeliex](https://github.com/yeliex) in
[https://github.com/swc-project/swc-node/pull/759](https://github.com/swc-project/swc-node/pull/759)
- fix(register): fix esm entry resolver for third-party executer, close
[#&#8203;762](https://github.com/swc-project/swc-node/issues/762) by
[@&#8203;yeliex](https://github.com/yeliex) in
[https://github.com/swc-project/swc-node/pull/766](https://github.com/swc-project/swc-node/pull/766)
- fix: support compile js files. close
[#&#8203;761](https://github.com/swc-project/swc-node/issues/761) by
[@&#8203;yeliex](https://github.com/yeliex) in
[https://github.com/swc-project/swc-node/pull/767](https://github.com/swc-project/swc-node/pull/767)
- chore: upgrade dependencies by
[@&#8203;Brooooooklyn](https://github.com/Brooooooklyn) in
[https://github.com/swc-project/swc-node/pull/771](https://github.com/swc-project/swc-node/pull/771)

**Full Changelog**:
https://github.com/swc-project/swc-node/compare/[@&#8203;swc-node/register](https://github.com/swc-node/register)[@&#8203;1](https://github.com/1).8.0...[@&#8203;swc-node/register](https://github.com/swc-node/register)[@&#8203;1](https://github.com/1).9.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNDAuNiIsInVwZGF0ZWRJblZlciI6IjM3LjM0MC42IiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiXX0=-->

Co-authored-by: tsang-bot[bot] <146107447+tsang-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

@swc-node/register@1.8.0 /esm-register does not recognise tsconfig.json paths
6 participants