No fallback for Register when entrypoint is a `commonjs` vscode extension

psychobolt commented 4 months ago

Hi, trying to execute my custom runtime with a vscode extension to support loading ESM files with register hook v1.10. However, I am getting this error in outputs:

[Info  - 10:13:43 AM] ESLint server is starting.
URL:file:///Users/mitran/.vscode/extensions/dbaeumer.vscode-eslint-3.0.10/server/out/eslintServer.js

node:internal/modules/run_main:129
    triggerUncaughtException(
    ^
Error: Failed to convert JavaScript value `Undefined` into rust type `String`
    at Compiler.<anonymous> (/Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-core-virtual-d7e358b2c0/node_modules/@swc/core/index.js:218:33)
    at Generator.next (<anonymous>)
    at /Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-core-virtual-d7e358b2c0/node_modules/@swc/core/index.js:31:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-core-virtual-d7e358b2c0/node_modules/@swc/core/index.js:27:12)
    at Compiler.transform (/Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-core-virtual-d7e358b2c0/node_modules/@swc/core/index.js:203:16)
    at transform (/Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-core-virtual-d7e358b2c0/node_modules/@swc/core/index.js:352:21)
    at transform (/Users/mitran/Projects/vite-storybook-boilerplate/.yarn/__virtual__/@swc-node-core-virtual-849eb160d1/3/.yarn/berry/cache/@swc-node-core-npm-1.13.1-7bd6e51ef2-10c0.zip/node_modules/@swc-node/core/lib/index.js:71:33)
    at compile (/Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-node-register-virtual-ef6475fd81/node_modules/@swc-node/register/lib/register.js:81:37)
    at load (file:///Users/mitran/Projects/vite-storybook-boilerplate/.yarn/unplugged/@swc-node-register-virtual-ef6475fd81/node_modules/@swc-node/register/esm/esm.mjs:183:28) {
  code: 'StringExpected'
}

Node.js v20.15.0

What would be executed for example would be ESLINT_USE_FLAT_CONFIG=true node bin/swc-node.js "../../.vscode/extensions/dbaeumer.vscode-eslint-3.0.10/server/out/eslintServer.js" -c eslint.config.ts.

Analysis

It seems that any commonjs file would cause the same error. For example I've created a simple test:

// foo.cjs
module.exports = {
  foo: 'bar'
};

node --import @swc-node/register ./foo.cjs

Would recieve the same error:

node:internal/modules/run_main:129
    triggerUncaughtException(
    ^
Error: Failed to convert JavaScript value `Undefined` into rust type `String`

Seeing here we shortcircuit and fails to compile when source is undefined. I would assume we need a similar fallback, which fixed the Yarn loader case . I have my workaround which works in my case:

diff --git a/esm/esm.mjs b/esm/esm.mjs
index 1e7429abec21338477f904b17a35161b75d01e19..f0a3db97959040ea925240ed0f7e38bba4dfb0e6 100644
--- a/esm/esm.mjs
+++ b/esm/esm.mjs
@@ -175,10 +175,14 @@ export const load = async (url, context, nextLoad) => {
         debug('loaded: internal format', url);
         return nextLoad(url, context);
     }
+    // workaround for a vscode extension, fallback. See https://github.com/microsoft/vscode/issues/130367
+    if (url.indexOf('.vscode/extensions') > 0) {
+        return nextLoad(url, context);
+    }
     const { source, format: resolvedFormat } = await nextLoad(url, context);

fargito commented 4 months ago

I have the same issue, however I think it is not specific to VSCode extension. I encounter it when I import a commonjs file with node. I opened #796 but I think my fix was incorrect, so I closed it.

fargito commented 4 months ago

I'm even wondering if commonjs files should be skipped altogether?

in packages/register/esm.mts

export const load: LoadHook = async (url, context, nextLoad) => {
  debug('load', url, JSON.stringify(context))

  if (url.startsWith('data:')) {
    debug('skip load: data url', url)

    return nextLoad(url, context)
  }

  if (['builtin', 'json', 'wasm'].includes(context.format)) {
    debug('loaded: internal format', url)
    return nextLoad(url, context)
  }

+  if (context.format === "commonjs") {
+    debug('skip load: commonjs file', url);
+    return nextLoad(url, context);
+  }

  const { source, format: resolvedFormat } = await nextLoad(url, context)

  debug('loaded', url, resolvedFormat)

  const code = !source || typeof source === 'string' ? source : Buffer.from(source).toString()
  const compiled = await compile(code, url, tsconfigForSWCNode, true)

  debug('compiled', url, resolvedFormat)

  return addShortCircuitSignal({
    // for lazy: ts-node think format would undefined, actually it should not, keep it as original temporarily
    format: resolvedFormat,
    source: compiled,
  })
}

WDYT @Brooooooklyn?

yeliex commented 4 months ago

I'm even wondering if commonjs files should be skipped altogether?

definitely no. import commonjs in esm is a very common case

I would assume we need a similar fallback, which fixed the Yarn loader https://github.com/swc-project/swc-node/issues/772#issuecomment-2094771696

A workaround should be. but ignoring by path prefix is too rude. I would look at this, maybe we could have a better way.

yeliex commented 4 months ago

could you please provide a minimal reproduction

psychobolt commented 4 months ago

Got a different error on this sandbox, but fails on commonjs script. Possibly related?

swc-project / swc-node

No fallback for Register when entrypoint is a `commonjs` vscode extension #795