vercel / next.js

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

ESM in .mjs files cause a dev mode runtime error #17806

Closed jaydenseric closed 6 months ago

jaydenseric commented 3 years ago

Bug report

Describe the bug

Importing a project ESM .mjs file (the Node.js standard file extension for ESM) in a page file causes a runtime error, but only in dev mode:

Unhandled Runtime Error ReferenceError: module is not defined

Screen Shot 2020-10-12 at 1 44 24 pm

Here is the call stack:

Screen Shot 2020-10-12 at 1 45 02 pm

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Create a basic Next.js project.
  2. Create a config.mjs file containing:

    export const A = 'a';
  3. In pages/index.js:

    import { A } from '../config.mjs';
    
    export default function IndexPage() {
     return A;
    }
  4. In Terminal, run:

    npm run dev

You should then see no error in Terminal, but on the client you will see the Next.js error overlay with the error described earlier.

Expected behavior

You should be able to use ESM .mjs files in your Next.js project without runtime errors in dev mode.

System information

Additional context

Other Next.js users have also encountered this issue:

https://github.com/vercel/next.js/discussions/13076

ScriptedAlchemy commented 3 years ago

https://twitter.com/ScriptedAlchemy/status/1315424669026058240?s=20

meglio commented 3 years ago

In the meantime, is there any workaround to use .mjs files in the dev mode? I'm trying to introduce Next.js to an existing project, but this prevents me from doing so. Please advise.

jerrygreen commented 3 years ago

Am I the single one here who doesn’t understand why choose mjs over js? Imports? Already supported in NextJS without having to use mjs. WASM modules? I already experimented with this and can assure you it’s also working already. Is it top level await or import.meta.url or what? P.S. no sarcasm btw, just wanna know

jaydenseric commented 3 years ago

@JerryGreen Next.js must support standard Node.js file extensions such as .mjs and .cjs without errors or bugs. If you are really interested in the design decisions behind the Node.js API for modules, read through the issues, PRs, and meeting notes of the Node.js Modules Team. Honestly, it's not worth the time for someone coming onto the scene now as there are many comments to read through; just follow the final rules as documented for the Node.js API.

Going further, Next.js should only support ESM in .js files if the project package.json contains "type": "module". If Next.js continues to refuse to adhere to Node.js standards regarding module formats I'll move to something else, or even create it myself if need be.

Already supported in NextJS without having to use mjs.

Just because Next.js has the ability to hack together webpack and Babel configs to make non standard things work due to a build step, doesn't mean it should. It's better for the ecosystem if all our tools work to common standards; e.g. you want to be able to use in a Next.js project the same ESLint config you use for linting imports in your published Node.js packages.

jerrygreen commented 3 years ago

Just because Next.js has the ability to hack together webpack and Babel configs to make non standard things work due to a build step, doesn't mean it should. It's better for the ecosystem if all our tools work to common standards

Oh, now I can finally see the reasoning! Yeah, I actually agree it should be supported, and less webpack hacks used: it's better to be handled by node since it does support all that too. Well, good thing Timer added this to the backlog milestone so hopefully it will be fixed sometime soon

JacobLey commented 3 years ago

I have recently run into this issue as well, using .js with "type":"module"

In my use case, NextJS is just a part of a larger monorepo, all written with ESM. I definitely support the comment

Just because Next.js has the ability to hack together webpack and Babel configs to make non standard things work due to a build step, doesn't mean it should. It's better for the ecosystem if all our tools work to common standards

In most cases it is great because it allows features like import/export and top-level-await, all without needing a special transpiler like Babel or Typescript (I still use Typescript, but with ES module/target, so no transpiling). But NextJS has trouble loading some of those utility files.

That twitter link https://github.com/vercel/next.js/issues/17806#issuecomment-707321843 actually appears to have fixed it for me, but it would be great to see this as a first-class feature and "just work" in NextJS without extra setup.

I will say I disagree with

Next.js should only support ESM in .js files if the project package.json contains "type": "module"

While that is most "correct", that would be breaking for most usage, which is writing ESM and expect it to automagically transpile into CommonJS.

If someone is deploying NextJS on its own (e.g. via Vercel, no custom server, no monorepo...) then Babels unambiguous sourceType is plenty acceptable to me

graup commented 3 years ago

The mentioned workaround also seems to have solved this issue for me.

Here's my config:

  webpack: (config) => {
    config.module.rules.push({
      test: /\.m?js$/,
      type: 'javascript/auto',
      resolve: {
        fullySpecified: false,
      },
    });
    return config;
  },

I'm using .ts files with { type: module } in package.json.

gpoole commented 2 years ago

Thanks @graup, worked around the issue for me on Next 11.1.3.

hutber commented 8 months ago

The mentioned workaround also seems to have solved this issue for me.

Here's my config:

  webpack: (config) => {
    config.module.rules.push({
      test: /\.m?js$/,
      type: 'javascript/auto',
      resolve: {
        fullySpecified: false,
      },
    });
    return config;
  },

I'm using .ts files with { type: module } in package.json.

Sadly with next 14 its moved to rust compiler so if you want to take advantage of the new speeds, we'll need to find another work around.

joulev commented 8 months ago

https://github.com/vercel/next.js/pull/58967 should already fix this I think, the workaround should no longer be required.

samcx commented 6 months ago

Hi everyone!

This seems to already be fixed on the latest, so I will be closing this one out!

If you are still coming across this issue, please feel free to send us a new bug report.

github-actions[bot] commented 6 months ago

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