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

Next server ESM support #25423

Closed
hanford opened this issue May 24, 2021 · 7 comments
Closed

Next server ESM support #25423

hanford opened this issue May 24, 2021 · 7 comments
Labels
bug Issue was opened via the bug report template.

Comments

@hanford
Copy link
Contributor

hanford commented May 24, 2021

What version of Next.js are you using?

10.2.2

What version of Node.js are you using?

15.14.0

What browser are you using?

n/a

What operating system are you using?

macOS

How are you deploying your application?

next export

Describe the Bug

When exporting my application, I now get an ESM module error:

~/dev/date-fns-test main*
❯ yarn build
yarn run v1.22.10
$ next build && next export
info  - Using webpack 5. Reason: no next.config.js https://nextjs.org/docs/messages/webpack5
info  - Checking validity of types
info  - Creating an optimized production build
info  - Compiled successfully

> Build error occurred
/Users/hanford/dev/date-fns-test/node_modules/date-fns/esm/format/index.js:1
import isValid from "../isValid/index.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at Object.compileFunction (node:vm:355:18)
    at wrapSafe (node:internal/modules/cjs/loader:1022:15)
    at Module._compile (node:internal/modules/cjs/loader:1056:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1121:10)
    at Module.load (node:internal/modules/cjs/loader:972:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Module.require (node:internal/modules/cjs/loader:996:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.259 (/Users/hanford/dev/date-fns-test/.next/server/pages/index.js:86:18)
    at __webpack_require__ (/Users/hanford/dev/date-fns-test/.next/server/webpack-runtime.js:25:42) {
  type: 'SyntaxError'
}
error Command failed with exit code 1.

This error occurs in date-fns land, it doesn't seem like a Next.js specific bug, though this used to not happen in previous versions of next and only began happening in version 10.2.1+ (I've verified this happens in 10.2.2 and next@canary).

When removing "module": "esm/index.js", from date-fns and date-fns-tz this error still occurs which is pretty strange.

I was able to get around this in my other production next app, by changing all esm files in date-fns to .mjs, which again, doesn't seem relevant to next.js, but this did used to work, so no idea!

Expected Behavior

My Next application should successfuly start/export

I understand this is likely a bug in date-fns or date-fns-tz, but this library used to work well in previous versions of Next and it no longer works

To Reproduce

yarn dev

OR

yarn build

Over here:
https://github.com/hanford/next-date-fns-bug

....

I couldn't think of a super accurate title here. Hoping a contributor or someone with context could rename the title 😬

@hanford hanford added the bug Issue was opened via the bug report template. label May 24, 2021
@hanford hanford changed the title Next ESM modules woes Next ESM module woes May 24, 2021
@mattcarlotta
Copy link
Contributor

mattcarlotta commented May 24, 2021

Problem

The server doesn't handle 3rd party ES modules; instead it expects 3rd party dependencies to be compiled to CJS (commonjs).

Solutions

You can get around this by either...

Option 1

Using next/dynamic and only doing CSR (client-side rendering):

components/example.tsx

import formatWithTimezone from "../utils/dates";

const date = formatWithTimezone(new Date(), "MMM d, yyyy hh:mma z");

export default function Example() {
  return <div style={{ padding: 24 }}>{date}</div>;
}

pages/index.tsx

import dynamic from "next/dynamic";

const FormatDate = dynamic(() => import("../components/example"), { ssr: false });

export default function Home() {
  return <FormatDate />;
}

Option 2

Or, better yet, transpiling the dependency to CJS for the server:

next.config.js

module.exports = {
  webpack(config, { defaultLoaders, isServer }) {
    if (isServer)
      config.module.rules.unshift({
        test: /\.js$/,
        loader: defaultLoaders.babel,
        include: /[\\/]node_modules[\\/](date-fns\/parseISO)[\\/]/,
      });

    return config;
  },
};

Other thoughts

This is not a bug with NextJS since it's server Webpack config is not/doesn't appear to be transpiling/handling 3rd party ESM dependencies (nor should it, as there may be compatibility issues), but instead a misstep?/miscalculation? with the library maintainers not factoring in that node's ES module support is still relatively new and is not universally adopted (on that note, all node versions prior to Node v13.2.0 wouldn't support ES modules -- at least not without the --experimental-module flag).

As of writing this, if I were to run this library on an API/web server running node, then the expected build output should/would be commonjs: example API or example web server. When you start using ES modules on the server, you'll quickly start running into compatibility issues with other libraries (for example, tsconfig-paths would have to support ESM to work with ts-node).

@advaiyalad
Copy link

Node v12 also supports ES modules. The last version to not support ES modules is unsupported (v10). Next.js should really support this, as ES modules are starting to be adopted widely. Some packages no longer support CJS. More and more people are asking for ES modules (just look at #9607).

I couldn't think of a super accurate title here. Hoping a contributor or someone with context could rename the title 😬

A good title would be "Next server ESM support".

People have said it before, and I am going to say it again: this has been part of the standards for years, and it has part of Node for a while. Why is NextJS not going to support it? Just because there "could be issues" doesn't mean that this shouldn't be implemented. Just because legacy/dead platforms like IE11 and node 10 don't support ESM, people who can/want to use modern platforms/things shouldn't be held back.

See #22381 as well.

@mattcarlotta
Copy link
Contributor

mattcarlotta commented May 26, 2021

Node v12 also supports ES modules.

Only versions v12.17.0+ and it's considerable experimental:

As of Node.js 12.17.0, the --experimental-modules flag is no longer necessary to use ECMAScript modules (ESM). However, the ESM implementation in Node.js remains experimental. As per our stability index: “The feature is not subject to Semantic Versioning rules. Non-backward compatible changes or removal may occur in any future release.” Users should be cautious when using the feature in production environments.

This means all versions prior to v12.17.0 wouldn't support ESM out of the box. While Node v10 is EOL, Node v12 is not: Node Versions.

Just because legacy/dead platforms like IE11 and node 10 don't support ESM, people who can/want to use modern platforms/things shouldn't be held back.

On the same token, a company whose product relies upon legacy dependencies/legacy browser support shouldn't have to worry about potentially breaking/updating their entire codebase for an experimental feature that hasn't quite become standard across all current versions of node. It's very easy to say: "Just add support," without factoring in how that change may adversely affect other dependencies. For example, how does adopting ESM on a custom Node server affect test suites that use jest?

As of now, Next still supports v10.13.0, but I have no doubt it'll fully support ESM in the near future. It just may take some time before full adoption occurs across the dependency tree/dev world. Until then, there appears to be at least some movement toward some ESM support: #22153

@advaiyalad
Copy link

Node 12.0.0 has rudimentary support for ES modules, see changelog. v12.17.0 just unflags the support. v12.20.0 (or is it v12.22.0?) makes the support stable.

On the same token, a company whose product relies upon legacy dependencies/legacy browser support shouldn't have to break and update their entire codebase

I see where you are coming from, and I understand that too. This could be hidden under a config option in next.config.js or be detected. There would be no effect for other users. This should not adversely affect anybody who doesn't want this feature. But there are people who are using packages that are pure ESM, or are converting their projects to pure ESM, and are having trouble even though it is standardized. It should work. But it doesn't.

Nonetheless, I think that there is no point arguing over who's right and who's not. Nobody's wrong. Everybody has something right to say. Trying to come up with a solution is more productive.

@hanford hanford changed the title Next ESM module woes Next server ESM support May 31, 2021
@hanford
Copy link
Contributor Author

hanford commented May 31, 2021

Thanks @PythonCreator27 and @mattcarlotta for the information.

This specific issue for me will be fixed in the next major version of date-fns which is nice, until then I've patched the library using https://www.npmjs.com/package/patch-package.

This isn't the first module that caused a little bit of ESM module headache and I'm sure it won't be the last.

I get why Next.js wouldn't support this use case, it isn't technically correct.

My date-fns and date-fns-tz patches are pretty straight forward, I had to rename all esm/** functions .mjs and tweak some import statements.

date-fns@2.18.0 patch
date-fns-tz@1.1.3 patch

@hos
Copy link

hos commented Oct 16, 2021

For those who are using yarn 2/3, here is an example with yarn patch.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

6 participants