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

Show better suggestions when CJS module resolution fails #26652

Open
Hajime-san opened this issue Oct 30, 2024 · 11 comments
Open

Show better suggestions when CJS module resolution fails #26652

Hajime-san opened this issue Oct 30, 2024 · 11 comments

Comments

@Hajime-san
Copy link
Contributor

Hajime-san commented Oct 30, 2024

Updated:

When importing CJS from ESM, there are cases where the module cannot be resolved.
In such cases, an appropriate message will be displayed so that the user can correct the code.

Node.js example below.

Deno's result.

% deno run app.tsx
error: Unable to load /path/to/Library/Caches/deno/npm/registry.npmjs.org/react-dom/16.8.5/server imported from file:///path/to/app.tsx

Caused by:
    No such file or directory (os error 2)

enivironment

% deno --version
deno 2.0.3 (stable, release, aarch64-apple-darwin)
v8 12.9.202.13-rusty
typescript 5.6.2

Steps to reproduce

  • deno.jsonc
{
  "imports": {
    "react": "npm:react@16.8.5",
    "react-dom": "npm:react-dom@16.8.5"
  },
  "compilerOptions": {
    "jsx": "react",
    "jsxFactory": "React.createElement",
    "jsxFragmentFactory": "React.Fragment"
  }
}
  • app.tsx
import { renderToStaticMarkup } from 'react-dom/server';

const App = () => renderToStaticMarkup(<h1>Hello, Deno!</h1>);

Old

It fails to import a slightly older version of React.
OTOH, it works fine with the latest(18.3.1) version.

 % deno run app.tsx             
error: Unable to load /path/to/Library/Caches/deno/npm/registry.npmjs.org/react/16.8.5/jsx-runtime imported from file:///path/to/app.tsx

Caused by:
    No such file or directory (os error 2)

enivironment

% deno --version
deno 2.0.3 (stable, release, aarch64-apple-darwin)
v8 12.9.202.13-rusty
typescript 5.6.2

Steps to reproduce

  • deno.jsonc
{
  "imports": {
    "react": "npm:react@16.8.5",
    "react-dom": "npm:react-dom@16.8.5"
  },
  "compilerOptions": {
    "jsx": "react-jsx",
    "jsxImportSource": "react"
  }
}
  • app.tsx
import { renderToStaticMarkup } from 'react-dom/server';

const App = () => renderToStaticMarkup(<h1>Hello, Deno!</h1>);

@rschristian
Copy link

rschristian commented Oct 31, 2024

react/jsx-runtime was added in v17.

In v16 and older, you need to use the classic JSX transform:

{
  "compilerOptions": {
    "jsx": "react",
    "jsxFactory": "React.createElement",
    "jsxFragmentFactory": "React.Fragment"
  }
}

You'll also need to add import React from 'react'; in each file that uses JSX.

@Hajime-san
Copy link
Contributor Author

Oops, thank you for pointing out!
However, it failure to import react-dom/server.🤔

% deno run app.tsx
error: Unable to load /path/to/Library/Caches/deno/npm/registry.npmjs.org/react-dom/16.8.5/server imported from file:///path/to/app.tsx

Caused by:
    No such file or directory (os error 2)

@nathanwhit
Copy link
Member

nathanwhit commented Oct 31, 2024

It looks like react-dom didn't have a ./server export back then, so it probably has to be react-dom/server.js. That file is CJS, and it looks like our static analysis can't figure out the exports, so you may have to use a default import, like

import server from "react-dom/server.js";

const { renderToStaticMarkup } = server;

@Hajime-san
Copy link
Contributor Author

Thanks, it works!
I'm not familiar with CJS module resolution in Deno, it could be nice if it had a suggestion with an error message.

MEMO:

@Hajime-san
Copy link
Contributor Author

The Import Maps is cool.

{
  "imports": {
    "react": "npm:react@16.8.5",
    "react-dom": "npm:react-dom@16.8.5",
+   "react-dom/server": "npm:react-dom@16.8.5/server.js"
  },
  "compilerOptions": {
    "jsx": "react-jsx",
    "jsxImportSource": "react"
  }
}

@bartlomieju
Copy link
Member

@Hajime-san is there anything actionable here for us then?

@Hajime-san
Copy link
Contributor Author

@bartlomieju
In my optimistic view, it would be better to have such a hint quoted from @nathanwhit suggestion when CJS module resolution fails.

error: Unable to load /path/to/Library/Caches/deno/npm/registry.npmjs.org/react-dom/16.8.5/server imported from file:///path/to/app.tsx

Caused by:
    No such file or directory (os error 2)

hint: The module that you want to import is CommonJS.
Consider adding the extension as shown below and going through the default import.

import cool from "foo/bar.js";
const { something } = cool;

BTW, I saw some PR is going that improves CJS module resolution now.
Will this problem be resolved?
cc @dsherret

@dsherret
Copy link
Member

dsherret commented Nov 1, 2024

No, this is not a bug. This is because this is ESM importing a CJS package. Similar errors occur in Node when importing from ESM:

> cat app.mjs
import { renderToStaticMarkup } from 'react-dom/server';

console.log(renderToStaticMarkup);
> node app.mjs
node:internal/modules/run_main:115
    triggerUncaughtException(
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module 'V:\scratch\example\node_modules\react-dom\server' imported from V:\scratch\example\app.mjs
Did you mean to import "react-dom/server.js"?
    at finalizeResolution (node:internal/modules/esm/resolve:260:11)
    at moduleResolve (node:internal/modules/esm/resolve:920:10)
    at defaultResolve (node:internal/modules/esm/resolve:1119:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:541:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:510:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:240:38)
    at ModuleJob._link (node:internal/modules/esm/module_job:126:49) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file:///V:/scratch/example/node_modules/react-dom/server'
}

Node.js v22.3.0

Then when importing with an extension:

> cat app.mjs
import { renderToStaticMarkup } from 'react-dom/server.js';

console.log(renderToStaticMarkup);
V:\scratch\example
> node app.mjs
file:///V:/scratch/example/app.mjs:1
import { renderToStaticMarkup } from 'react-dom/server.js';
         ^^^^^^^^^^^^^^^^^^^^
SyntaxError: Named export 'renderToStaticMarkup' not found. The requested module 'react-dom/server.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'react-dom/server.js';
const { renderToStaticMarkup } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:171:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:254:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:474:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5)

Node.js v22.3.0

And finally solved by using the default import:

> cat app.mjs
import server from 'react-dom/server.js';

console.log(server.renderToStaticMarkup);
V:\scratch\example
> node app.mjs
[Function: renderToStaticMarkup]

@dsherret
Copy link
Member

dsherret commented Nov 1, 2024

I think it would be nice if Deno suggested to import server.js similar to node, as you suggested and then to suggest to use the default import in case it's not doing it.

@Hajime-san
Copy link
Contributor Author

@dsherret
Thanks a lot!

@Hajime-san Hajime-san changed the title Failed to import a little bit old React Show better suggestions when CJS module resolution fails Nov 1, 2024
@Hajime-san
Copy link
Contributor Author

Note: Former error is not a JsError.

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

No branches or pull requests

5 participants