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

Module resolution errors #161

Closed
lionelhorn opened this issue Nov 30, 2021 · 18 comments · May be fixed by #302
Closed

Module resolution errors #161

lionelhorn opened this issue Nov 30, 2021 · 18 comments · May be fixed by #302

Comments

@lionelhorn
Copy link
Contributor

I have a NextJs app inside a monorepo. Pacakges are built using Nx

package.json

"dependencies": {
    "@deepkit/type": "^1.0.1-alpha.58",
    [...]
}

Yarn install log

yarn install v1.22.17
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning " > @deepkit/type@1.0.1-alpha.58" has unmet peer dependency "@deepkit/core@^1.0.1-alpha.13".
warning " > @deepkit/type@1.0.1-alpha.58" has unmet peer dependency "reflect-metadata@^0.1.13".
[...]

I try to build

nx build

I get

../../node_modules/@deepkit/type/dist/esm/src/decorators.js
Module not found: Can't resolve '@deepkit/core' in '[path-omitted]\node_modules\@deepkit\type\dist\esm\src'

Looking at the yarn log, I then see that It's messing a peer dependency to
"@deepkit/core@^1.0.1-alpha.13"
"reflect-metadata": "^0.1.13"

So I added those.

Then the build error becomes

Cannot find module '[path-omitted]\node_modules\@deepkit\type\dist\esm\src\types' imported from [path-omitted]\node_modules\@deepkit\type\dist\esm\index.js

Any ideas?
@marcj
Copy link
Member

marcj commented Nov 30, 2021

Do the files in node_modules\@deepkit\type\dist\esm\src exist?

@lionelhorn
Copy link
Contributor Author

node_modules\@deepkit\type\dist\esm\src exists.

Can the layout of the monorepo be an issue?
There is only one node_modules and one package.json for the whole repo at the root level and Nx is supposed to detect the dependency used by each package.

├───apps
│   ├───kds
│   │   ├───src
│   │   │   ├───components
│   │   │   ├───core
│   │   │   │   ├───data
│   │   │   │   ├───orders
│   │   │   │   ├───transform
│   │   │   │   └───ui
│   │   │   ├───styles
│   │   │   ├───utils
│   │       └───views
├───libs
│   └───shared
│       └───src   
├───node_modules
│   ├───@deepkit
│   │   ├───core
│   │   │   ├───dist
│   │   │   │   ├───cjs
│   │   │   │   │   ├───src
│   │   │   │   │   └───tests
│   │   │   │   └───esm
│   │   │   │       ├───src
│   │   │   │       └───tests
│   │   │   ├───node_modules
│   │   │   │   └───to-fast-properties
│   │   │   ├───src
│   │   │   └───tests
│   │   └───type
│   │       ├───dist
│   │       │   ├───cjs
│   │       │   │   └───src
│   │       │   └───esm
│   │       │       └───src
│   │       └───node_modules
│    package.json   

@marcj
Copy link
Member

marcj commented Nov 30, 2021

Does your tree structure mean there are no files at all in it? It looks for a file node_modules\@deepkit\type\dist\esm\src\types.js. is this available?

@lionelhorn
Copy link
Contributor Author

Yes, the node_modules\@deepkit\type\dist\esm\src\types.js is present

What I tried to illustrate in the tree, is that there no separate node_modules for each "package".

@marcj
Copy link
Member

marcj commented Nov 30, 2021

That's fine. I use monorepos all the time, so that shouldn't be a problem. So the file exists but your build tool complains it does not exist. This is unfortunately not related to deepkit as the only thing we need to make sure is that the files are available and they correctly import each other. Anything else is up to the build tool.

Does node_modules\@deepkit\type\dist\esm\index.js have the correct import to the type file?

@lionelhorn
Copy link
Contributor Author

Nx calls nextjs build tool
So I get the same error using
npx next build

If I understood your question correctly

node_modules\@deepkit\type\dist\esm\index.js

export * from './src/types';

@marcj
Copy link
Member

marcj commented Nov 30, 2021

Ok, that code looks fine. So, it seems you installed all the packages correctly and the files are correct. At this point, I can't help further as I have no idea how next builds the stuff and how to debug that error using it. I'd probably try to modify the file node_modules\@deepkit\type\dist\esm\index.js a bit and check if that has any implication and if so which one (so to check if that is really the actual read/used file for example)

@lionelhorn
Copy link
Contributor Author

Thanks.
I really appreciate the pointers on how to fix the issue :) !

On a side note, I see this project is using Lerna.
Have you used others in the past or thinking of migrating to others?

I've seen Lerna isn't actively maintained anymore.
lerna/lerna#2703 (comment)

@marcj
Copy link
Member

marcj commented Nov 30, 2021

Have you used others in the past or thinking of migrating to others?

Yeah, was thinking about using Rush from Microsoft, but nothing concretely planned. Lerna works well for the moment still. :)

@lionelhorn
Copy link
Contributor Author

lionelhorn commented Nov 30, 2021

I posted the issue at vercel/next.js#31974

TL;DR: Fails with NextJS 12.x. Works with NextJS 11.x

@lionelhorn
Copy link
Contributor Author

lionelhorn commented Nov 30, 2021

Could you have a look at
vercel/next.js#31974 (comment)
for a possible explanation on the underlying issue.

"when available. node_modules are directly loaded by Node.js and not bundled, so the Node.js ESM guidelines apply."

@lionelhorn lionelhorn reopened this Nov 30, 2021
@marcj
Copy link
Member

marcj commented Dec 1, 2021

TypeScript's compiler does not support Node's default ESM resolution algorithm. They have introduced it in version 4.5 beta https://devblogs.microsoft.com/typescript/announcing-typescript-4-5-beta/#esm-nodejs, but dropped it in the final 4.5 due to issues. To enable the commonly used module resolution algorithm you have to pass to node --experimental-specifier-resolution=node. You can also configure that in your environment variable NODE_OPTIONS=--experimental-specifier-resolution=node. If nextjs supports passing custom node arguments, you can configure it there.

@productdevbook
Copy link

some problem next 12

and dont fixed -> NODE_OPTIONS=--experimental-specifier-resolution=node

image

@JumpLink
Copy link

JumpLink commented Jul 12, 2022

I think it would help to add the .js file extension on all internal imports to fix the ESM packages, this should not break the CJS packages.

@marcj
Copy link
Member

marcj commented Jul 12, 2022

We added already a lot of .js file extension in our imports, not all are covered yet though

@JumpLink
Copy link

@marcj Okay, here a MR #302 to add the missing file extensions, too :)

@productdevbook
Copy link

if that means it will work with nuxt 3 as well ?

@JumpLink
Copy link

JumpLink commented Jul 18, 2022

@productdevbook If nuxt / next uses ESM and the file extensions were the only problem, then yes, but I don't know if there are other problems, we will see once the PR is merged and a new release is released

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 a pull request may close this issue.

4 participants