vercel / next.js

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

libraries using `__dirname` don't resolve correctly in server components #49783

Closed mPaella closed 1 year ago

mPaella commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: win32
      Arch: x64
      Version: Windows 10 Pro
    Binaries:
      Node: 16.17.1
      npm: N/A
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 13.4.3-canary.0
      eslint-config-next: 13.3.1
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.2

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true)

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/gifted-einstein-mo56ss?file=%2Fnext.config.js%3A19%2C7

To Reproduce

Describe the Bug

The reproduction uses a library, @emurgo/cardano-sererialization-lib that's compiled with wasm-pack. Wasm-pack saves the .WASM file, and loads it by deriving the path using this compiled code:

const path = require('path').join(__dirname, 'cardano_serialization_lib_bg.wasm');

As a hack to see if i could fix it, I edited the compiled lib to use this, which works correctly:

const path = require('path').join(process.cwd(), "node_modules", "@emurgo", "cardano-serialization-lib-nodejs", 'cardano_serialization_lib_bg.wasm');

Expected Behavior

I expect it to "just work" in both the app and pages directory

This issue could be fixed by the underlying libraries like wasm-pack, and I originally opened an issue there, but it seems like this may be a larger problem with nextjs itself

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

JesseKoldewijn commented 1 year ago

In the case you're using typescript you probably don't have the esModuleInterop enabled. Could you check if this fixes your issue by chance? https://www.typescriptlang.org/tsconfig#esModuleInterop

mPaella commented 1 year ago

@JesseKoldewijn we do already have that in our tsconfig

{
    "compilerOptions": {
        "experimentalDecorators": true,
        "emitDecoratorMetadata": true,
        "target": "es5",
        "lib": ["dom", "dom.iterable", "esnext"],
        "allowJs": true,
        "skipLibCheck": true,
        "strict": true,
        "forceConsistentCasingInFileNames": true,
        "noEmit": true,
        "esModuleInterop": true,
        "module": "esnext",
        "moduleResolution": "node",
        "resolveJsonModule": true,
        "isolatedModules": true,
        "jsx": "preserve",
        "incremental": true,
        "baseUrl": ".",
        "paths": {
            "@/components/*": ["components/*"],
            "@/config/*": ["config/*"],
            "@/utils/*": ["utils/*"],
            "@/layouts/*": ["layouts/*"],
            "@/storage/*": ["storage/*"],
            "@/models/*": ["models/*"],
            "@/services/*": ["services/*"],
            "@/types/*": ["types/*"],
            "@/modules/*": ["modules/*"],
            "@/consts/*": ["consts/*"],
            "@/hooks/*": ["hooks/*"],
            "@/styles/*": ["styles/*"],
            "@/classes/*": ["classes/*"],
            "@/pages/*": ["pages/*"],
            "@/emailTemplates/*": ["emailTemplates/*"],
            "@/cypress/*": ["cypress/*"],
            "@/blockchain/*": ["blockchain/*"],
            "@/middleware/*": ["middleware/*"],
            "@/tests/*": ["tests/*"]
        },
        "plugins": [
            {
                "name": "next"
            }
        ]
    },
    "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", "types/.d.ts", "migrations/*.ts", ".next/types/**/*.ts"],
    "exclude": ["node_modules"],
    "ts-node": {
        "require": ["tsconfig-paths/register"],
        "compilerOptions": {
            "module": "CommonJS"
        }
    }
}
JesseKoldewijn commented 1 year ago

Does do you yourself give it a path or does it do this internally in the package itself?

mPaella commented 1 year ago

The path is generated by wasm-pack, so inside the library when it's built/released

You can view it in the compiled bundle (go here => click "code" => open cardano_serialization_lib.js => go to line 27629)

image
JesseKoldewijn commented 1 year ago

Alright check! Could you maybe try the following.

In your Next.js config, add the "@emurgo/cardano-serialization-lib-nodejs" to the "experimantal" config entry "serverComponentsExternalPackages" to check if it still throws this same error when added to this config option.

mPaella commented 1 year ago

In your Next.js config, add the "@emurgo/cardano-serialization-lib-nodejs" to the "experimantal" config entry "serverComponentsExternalPackages" to check if it still throws this same error when added to this config option.

Ah i see, didn't know that was an option. That does appear to be working after altering the reproduction.

I will try in our production app and see if its working there as well.

mPaella commented 1 year ago

@JesseKoldewijn I can confirm adding the libraries to experimental.serverComponentsExternalPackages has solved the problem for me

JesseKoldewijn commented 1 year ago

I'll open a PR tomorrow that fixes this issue in the future👌

JesseKoldewijn commented 1 year ago

@mPaella did you only add the package in question to that config entry or also wasm-pack?

mPaella commented 1 year ago

I went with this

serverComponentsExternalPackages: ["@blockfrost/blockfrost-js", "@jpg-store/lucid-cardano", "mongoose"],
JesseKoldewijn commented 1 year ago

@mPaella just submitted the PR ;) So if everything goes smoothly you'll be able to run your project on the canary release after this PR gets merged without the before mentioned config entry 👍

mPaella commented 1 year ago

Awesome!

Would love to know what the longer term solution here is as well. What's the difference in compilation that's causing these packages to be added to the exclusions list?

What can libraries do to avoid this?

JesseKoldewijn commented 1 year ago

Awesome!

Would love to know what the longer term solution here is as well. What's the difference in compilation that's causing these packages to be added to the exclusions list?

What can libraries due to avoid this?

As far as I know it has to do with how the App Router is compiled. As for how to prevent this, not sure. Haven't dug that deep into it to be honest. Plus I personally haven't ran into a package in the projects that I work on where they run into these compile issue. I mainly recognized this behavior because bcrypt also used to have issues in the App Router before I PR'ed it into this external package list. ;)

But, I'm going to move to a Next.js focused job soon so I'm sure I'll be of more used by that time with regards to more deep stuff than just these small fixes which are just pattern recognition in my mind right now.

JesseKoldewijn commented 1 year ago

@mPaella btw, can you keep this issue open by chance till its fixed upstream without the experimental manual config?

JesseKoldewijn commented 1 year ago

The PR unfortunately is still open in review (just synced it with upstream to make merging for the core team easier). So keep an eye on PR #49938 I would expect it can be merged soon since its just a two-line addition.

JesseKoldewijn commented 1 year ago

@mPaella the fix for your issue just got merged. So make sure to bump your project to the next canary release (as in, the next release from this moment) to be able to run your project without the before mentioned manual experimental config👌

Edit: just checked real quick and it's already pushed into the latest canary release🤙

github-actions[bot] commented 1 year ago

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.