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

esbuild fails on Generating static pages step in serverless Next.js applications #14

Closed
t3dotgg opened this issue Mar 25, 2021 · 14 comments · Fixed by #21
Closed

esbuild fails on Generating static pages step in serverless Next.js applications #14

t3dotgg opened this issue Mar 25, 2021 · 14 comments · Fixed by #21
Labels
Next.JS Issue with using mdx-bundler with Next.JS released

Comments

@t3dotgg
Copy link

t3dotgg commented Mar 25, 2021

  • mdx-bundler version: 3.1.2
  • node version: any (tried 12, 14, and 15.12)
  • npm version: 7.6.3

Relevant code or config: repl.it of bug

What you did: Added target: "serverless" to a next.js project with mdx-bundler

What happened:
image

Reproduction repository: https://replit.com/@TheoBr/mdx-bundler-static-demo#README.md

Problem description: Seems to be an issue with how esbuild is run in next.js's serverless path. Not functionality I'm super familiar with, I just know that netlify forces it to be enabled. (fwiw I've had no issues using this with vercel (see: tt.fm)

Suggested solution: Bug EvanW or Guillermo maybe? I'm lost on this one

@Arcath
Copy link
Collaborator

Arcath commented Mar 25, 2021

I had issues with Next.JS / Webpack breaking __dirname which stops esbuild from finding its binary.

You can set ESBUILD_BINARY_PATH in your env and it will over-ride auto detection.

I use this in my Next.JS site:

if(process.platform === "win32"){
  process.env.ESBUILD_BINARY_PATH = path.join(process.cwd(), 'node_modules', 'esbuild', 'esbuild.exe')
}else{
  process.env.ESBUILD_BINARY_PATH = path.join(process.cwd(), 'node_modules', 'esbuild', 'bin', 'esbuild')
}

Write up: https://www.arcath.net/2021/03/mdx-bundler

@Arcath Arcath added the Next.JS Issue with using mdx-bundler with Next.JS label Mar 25, 2021
@kentcdodds
Copy link
Owner

@Arcath could you add a link to that on the README somewhere?

Arcath added a commit that referenced this issue Mar 25, 2021
@kentcdodds
Copy link
Owner

You know what... The more I think about this, the more I think we should probably support this out of the box 🤔 We know where the esbuild binary should be, so we can set that env variable ourselves if it's not already set.

@Arcath
Copy link
Collaborator

Arcath commented Mar 26, 2021

@kentcdodds I was thinking the same thing.

I was thinking this is all it would need:

if(__dirname === "" && !process.env.ESBUILD_BINARY_PATH){
  //FIX
}

That way we fix it if we need to and don't interferrer with anyones overrides

@malcolm-kee
Copy link

In case anyone also using this with NextJS in netlify, somehow __dirname equals to / during build.

My workaround is this:

if (process.env.NETLIFY && !process.env.ESBUILD_BINARY_PATH) {
    if (process.platform === 'win32') {
      process.env.ESBUILD_BINARY_PATH = path.join(
        process.cwd(),
        'node_modules',
        'esbuild',
        'esbuild.exe'
      );
    } else {
      process.env.ESBUILD_BINARY_PATH = path.join(
        process.cwd(),
        'node_modules',
        'esbuild',
        'bin',
        'esbuild'
      );
    }
  }

@kentcdodds
Copy link
Owner

@Arcath, I'm not sure I understand how __dirname would ever be '' 🤔

@kentcdodds
Copy link
Owner

Actually, now that I understand this a bit more, this seems like a bug in webpack/next. Could we get it fixed there instead? __dirname should totally work for those environments.

@Arcath
Copy link
Collaborator

Arcath commented Apr 9, 2021

@Arcath, I'm not sure I understand how __dirname would ever be '' 🤔

Thats what it came out as in my testing, I added console.dir(__dirname) to the offending function in esbuild.

Actually, now that I understand this a bit more, this seems like a bug in webpack/next. Could we get it fixed there instead? __dirname should totally work for those environments.

I came across this vercel/next.js#14341 (comment) where they are implying that removing __dirname is "working as intended" as the bundled serverless functions would be given the absolute path at build time and not the run time path. Setting it to '' should cause build errors that you can fix by using process.cwd().

@kentcdodds
Copy link
Owner

I'm just worried that the solution will be too magical. I'd rather folks do the extra work themselves. We can make the issue and fix more prominent in the docs if that helps though.

@Arcath
Copy link
Collaborator

Arcath commented Apr 12, 2021

Maybe if we check if our __dirname includes mdx-bundler and if not output a message to console saying "Esbuild maybe unable to find its binary, if your build fails see LINK_TO_REAMDE"

@kentcdodds
Copy link
Owner

Yeah, I'd be willing to accept a PR for that 👍

@kentcdodds
Copy link
Owner

Oh... Actually that might be tricky and it's hard to explain. I'll build this myself.

@kentcdodds
Copy link
Owner

Reviews welcome: #21

kentcdodds added a commit that referenced this issue Apr 13, 2021
* feat: warn when __dirname is messed up

Closes #14

* fix types

* don't warn if ESBUILD_BINARY_PATH is already set
@github-actions
Copy link

🎉 This issue has been resolved in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Next.JS Issue with using mdx-bundler with Next.JS released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants