vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.84k stars 26.65k forks source link

Build issue with subpath imports of internal "compiled" package #67095

Closed OllieJennings closed 3 weeks ago

OllieJennings commented 2 months ago

Link to the code that reproduces this issue

https://codesandbox.io/p/github/shaunjohann/turbo-starter/main

See https://github.com/vercel/next.js/issues/67095#issuecomment-2288091176

To Reproduce

  1. In the repo above run next dev and everything will run fine.
  2. Run next build and the build with error

Below is for doing it locally

  1. Define subpath imports for an internal package in a turborepo monorepo:
"imports": {
    "#utils/*": "./src/utils/*",
    "#organization/*": "./src/organization/*",
    "#organization-role/*": "./src/organization-role/*"
  },
  1. Utilize this in some file: import { StringOrDateSchema, timezone } from "#utils/index.js";

  2. Compile internal package via tsc into internal "compiled" package

  3. Run next dev in a nextjs project which deps on the above, which will run perfectly fine (no issues)

  4. Run next build which throws the issue:

Failed to compile.
Module not found: Can't resolve '#utils/index.js'

Current vs. Expected behavior

Current Behaviour: nextjs will run the dev server just fine, but any build will fail.

Expected Behaviour: it should just work.

As per the docs of turborepo, you recommend subpath imports over tsconfig paths.

However it seems like nextjs does not support these.

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP Fri Mar 29 23:14:13 UTC 2024
  Available memory (MB): 31698
  Available CPU cores: 32
Binaries:
  Node: 21.0.0
  npm: 10.2.0
  Yarn: N/A
  pnpm: 9.3.0
Relevant Packages:
  next: 14.2.4 // Latest available version is detected (14.2.4).
  eslint-config-next: N/A
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.4.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Module Resolution

Which stage(s) are affected? (Select all that apply)

next build (local), Vercel (Deployed)

Additional context

No response

boylec commented 1 month ago

I'm getting the same behavior with webpack on Next.JS 14.2.5

I can run next dev, I can run storybook, I cannot run next build. Seems to not be honoring subpath import in my package.json as per the OP's description.

OllieJennings commented 1 month ago

Summoning @leerob or @timneutkens as l think the turbo-repo docs are recommending something that nextjs doesn't support yet:

E.g. https://turbo.build/repo/docs/guides/tools/typescript#use-nodejs-subpath-imports-instead-of-typescript-compiler-paths

lubieowoce commented 1 month ago

Hey @OllieJennings, thanks for opening the issue. I can't find an "imports" field or usage of "#utils/index.js" anywhere in the linked codesandbox. Could you please update the code to reproduce the issue you're describing? Thanks.

(Also removing some of packages not relevant to the repro would be appreciated, that starter contains a lot of code)

OllieJennings commented 1 month ago

@lubieowoce let me re-create one, it seems like my sandbox link is now broken somehow, will do it within the next hr

OllieJennings commented 4 weeks ago

@lubieowoce i almost have the full repo working: https://codesandbox.io/p/github/BJR-developer/basic-turborepo/csb-25k8gf/subpath-imports-repo.

Just a few tweaks but it should help you get started on debugging

lubieowoce commented 4 weeks ago

Short explanation

I believe this is not a Next.js issue, but an incorrectly configured package. You see, imports aliasing acts a bit unintuitively for packages compiled using tsc, where code is written in src/ but runs from dist/. Because when you alias "#utils/*": "./src/utils/*", it makes the compiled JS code from dist try to import code from src, which obviously isn't gonna work.

Instead, you should alias "#utils/*": "./dist/utils/*". See also examples in the TS docs, showing that imports should point to files in dist, not in src. This looks a bit strange, but works fine, including (in my testing) VSCode's Intellisense.


Long explanation

If you compile @repo/test-pkg and try to import it in Node, it fails too (proving this isn't Next.js specific)

$ pnpm turbo build --filter='@repo/test-pkg'
<snip>
$ ( cd apps/web && node -e 'import("@repo/test-pkg/somefile").then((m) => m.somefile())' )
node:internal/modules/esm/resolve:264
    throw new ERR_MODULE_NOT_FOUND(
          ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '<REPO>/packages/test-pkg/src/utils/some-thing.js' imported from <REPO>/packages/test-pkg/dist/some-file.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 ModuleLoader.defaultResolve (node:internal/modules/esm/loader:396:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:365:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:240:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:85:39)
    at link (node:internal/modules/esm/module_job:84:36) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file:///<REPO>/packages/test-pkg/src/utils/some-thing.js'
}

If read the error closely, we'll see that this is because dist/some-file.js still contains the "#utils/some-thing.js" import...

import { helloSomething } from "#utils/some-thing.js";
export function somefile() {
    helloSomething();
    console.log("IN HERE");
    return "return from some file";
}

and out imports look like this...

  "imports": {
    "#utils/*": "./src/utils/*"
  },

...so the import alias in package.json makes #utils/some-thing.js point to src/utils/some-thing.js, which doesn't exist!

For the compiled output to work, we should be aliasing to dist/utils/some-thing.js instead:

  "imports": {
    "#utils/*": "./dist/utils/*"
  },

After making this change, the node test from above will run fine.

$ ( cd apps/web && node -e 'import("@repo/test-pkg/somefile").then((m) => m.somefile())' )
Hello from something
IN HERE

Some ideas for workarounds

So this is a bit annoying, and honestly, I'm not sure what the right way to work around it is. Of course you can just make imports point into dist, but that'll probably require running a tsc --watch to have editor stuff work right.

Another option would be to use a build tool or typescript plugin that resolves "#aliased/paths" when compiling, so that the output in dist no longer uses those. I'm not sure what the options here are. Maybe something like https://github.com/LeDDGroup/typescript-transform-paths -- they primarily deal with tsconfig.json#compilerOptions.paths, but maybe they do package.json#imports too.

EDIT: actually, it looks like even with "#utils/*": "./dist/utils/*", VSCode's go-to-definition correctly navigates to the file in src/ instead, so we don't really need any workarounds for this!

lubieowoce commented 4 weeks ago

@OllieJennings does that work for you?

OllieJennings commented 4 weeks ago

@lubieowoce l feel like a bit of an idiot, it seems to be working, but am just testing by migrating some of my internal packages in my monorepo to this.

lubieowoce commented 3 weeks ago

Okay, I'm gonna close this, let us know if you find any Next.js-specific problems with this

nathan1530 commented 3 weeks ago

@lubieowoce But the main point of Just-in-Time Packages is not to build/tranpile the tsx or ts files. So directly export typescript from src/* should work.

lubieowoce commented 3 weeks ago

@nathan1530 If you're using Next.js's transpilePackages, then "imports": { "#utils/*": "./src/utils/*" } works fine. In fact that's the only one that will work, because no dist folder will be created (unlike tsc).

Maybe we should consider supportingimports that point to files in tsconfig.json#compilerOptions.outDir even if they don't exist on disk -- TSC already seems to resolve things this way when typechecking. It's unfortunate that currently there seems to be no good way of writing a package.json with imports that'll be valid both with tsc and transpilePackages (the situation with exports is similar AFAICT)

lubieowoce commented 3 weeks ago

I've also forwarded this to the Turborepo team, so their docs should include some info about this soon :)

geclos commented 2 weeks ago

Hi,

I'm pretty sure next build does not support subpath imports. I've done this example repo to demonstrate it.

Should I open a new issue?

lubieowoce commented 1 week ago

@geclos Hmm. I looked at your repo, and AFAICT your issue only appears if using transpilePackages on a package that uses #imports. if we disable transpilePackages and convert packages/ui to be compiled with tsc (as described in this issue btw), then next build runs fine.

What to change to see this ```diff diff --git a/apps/web/next.config.mjs b/apps/web/next.config.mjs index 6dcda59..4678774 100644 --- a/apps/web/next.config.mjs +++ b/apps/web/next.config.mjs @@ -1,6 +1,4 @@ /** @type {import('next').NextConfig} */ -const nextConfig = { - transpilePackages: ['@repo/ui'] -}; +const nextConfig = {}; export default nextConfig; diff --git a/packages/ui/package.json b/packages/ui/package.json index 0e51c09..06284ad 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -3,14 +3,15 @@ "version": "0.0.0", "private": true, "exports": { - "./button": "./src/button.tsx", - "./card": "./src/card.tsx", - "./code": "./src/code.tsx" + "./button": "./dist/button.js", + "./card": "./dist/card.js", + "./code": "./dist/code.js" }, "imports": { - "#*": "./src/*" + "#*": "./dist/*" }, "scripts": { + "build": "tsc", "lint": "eslint . --max-warnings 0", "generate:component": "turbo gen react-component" }, diff --git a/packages/ui/tsconfig.json b/packages/ui/tsconfig.json index ca86687..b21e74a 100644 --- a/packages/ui/tsconfig.json +++ b/packages/ui/tsconfig.json @@ -1,6 +1,7 @@ { "extends": "@repo/typescript-config/react-library.json", "compilerOptions": { + "rootDir": ".", "outDir": "dist" }, "include": ["src"], ``` (to apply this change easily, you can download this file: [compile-ui-with-tsc.patch](https://github.com/user-attachments/files/16886390/compile-ui-with-tsc.patch), and `git apply PATH_TO_THAT_FILE` in the root of your repo)

So yeah, I'd open a separate issue regarding transpilePackages specifically. Thanks!

(Also, please remove redundant packages/apps like apps/docs from the repro, it makes our work easier)