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

SyntaxError: Unexpected token 'export' #31518

Closed
alienbuild opened this issue Nov 16, 2021 · 16 comments
Closed

SyntaxError: Unexpected token 'export' #31518

alienbuild opened this issue Nov 16, 2021 · 16 comments
Labels
bug Issue was opened via the bug report template.

Comments

@alienbuild
Copy link

What version of Next.js are you using?

12.0.4

What version of Node.js are you using?

12.22.0

What browser are you using?

Chrome @latest

What operating system are you using?

Windows 10

How are you deploying your application?

Manual

Describe the Bug

Nextjs is failing on build of a third party package. The third party package is using ES6 syntax for exports.

export { Root } from "./.internal/core/Root";
^^^^^^

SyntaxError: Unexpected token 'export'
    at wrapSafe (internal/modules/cjs/loader.js:915:16)
    at Module._compile (internal/modules/cjs/loader.js:963:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.9504 (C:\_lab\***\.next\server\pages\***\market\components\TradingChart\PrimaryChart.js:66:18)
    at __webpack_require__ (C:\_lab\***\.next\server\webpack-runtime.js:25:42)
    at Object.9084 (C:\_lab\***\.next\server\chunks\9084.js:17:77) {
  type: 'SyntaxError'
}

I appreciate this is perhaps more of an issue with the third party package but interested to know if there's a way I can support it in my build ?

Expected Behavior

Expectation is a smooth build regardless of syntax used

To Reproduce

Install latest nextjs and amcharts5 and run build

@alienbuild alienbuild added the bug Issue was opened via the bug report template. label Nov 16, 2021
@pmerolle

This comment has been minimized.

@balazsorban44
Copy link
Member

The package is using ESModules which Next.js 12 supports out of the box, but it's probably missing something, maybe "type": "module" in its package.json and Next tries to handle it as a Common JS module.

You should reach out to the third party package so they can investigate their bundling.

@Pauan
Copy link

Pauan commented Dec 12, 2021

@balazsorban44 I am speaking on behalf of amCharts. Our package does indeed use ES6 modules, and it works perfectly everywhere else: Webpack, Rollup, React, Angular, Vue, etc. This problem is exclusively with Next.js.

It's possible to workaround it by transpiling amCharts like this:

const withTM = require('next-transpile-modules')(['@amcharts/amcharts5'])

module.exports = withTM({});

But that shouldn't be necessary, everything should work out of the box. "type": "module" is not needed with any other build system, that is only needed if you are running the ES6 modules directly in Node. If you use "type": "module" to detect ES6 modules, you will break countless thousands of packages (which work correctly everywhere else).

@balazsorban44
Copy link
Member

balazsorban44 commented Dec 12, 2021

Check out our blog post https://nextjs.org/blog/next-12#es-modules-support-and-url-imports

Next 12 prefers the standardized method. I can also recommend the Node.js docs around ESM:

https://nodejs.org/docs/latest/api/esm.html

If you are not ready yet, you can try to temporarily switch back to prefer CJS, using

experimental: {
  esmExternals: false
}

in next.config.js I believe.

https://nextjs.org/blog/next-11-1#es-modules-support

@Pauan
Copy link

Pauan commented Dec 12, 2021

@balazsorban44 I don't see anything in that blog about "type": "module" (which has never been needed, and is still not needed by any build systems).

Using "type": "module" is not standard, it is Node.js only, and it is not used by any JS build systems.

As I said, if you are requiring "type": "module" you will break many thousands of ES6 packages which work correctly with every other build system.

This has nothing to do with ES6 vs CommonJS, our code uses 100% standard ES6 modules and has worked for years in every other build system. This is a Next.js specific issue.

@sokra
Copy link
Member

sokra commented Dec 13, 2021

Next.js (in all versions) uses Node.js to execute code for SSR or in API routes. User code (code not in node_modules) will be bundled by webpack, but non user code (code in node_modules) will not be processed in any way and just required resp. imported by Node.js. This means any code you import from node_modules need to be compatible with Node.js.

@amcharts/amcharts5 is not compatible with Node.js. You can run in Node.js.

Here is how to try it

Neither importing:

> node -e "import('@amcharts/amcharts5').then(m => console.log(m))"
(node:12204) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
C:\Repos\...\node_modules\@amcharts\amcharts5\index.js:1
export { Root } from "./.internal/core/Root";
^^^^^^

SyntaxError: Unexpected token 'export'
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1031:15)
    at Module._compile (node:internal/modules/cjs/loader:1065:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:190:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:185:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)

nor requireing works:

> node -e "console.log(require('@amcharts/amcharts5'))"
C:\Repos\...\node_modules\@amcharts\amcharts5\index.js:1
export { Root } from "./.internal/core/Root";
^^^^^^

SyntaxError: Unexpected token 'export'
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1031:15)
    at Module._compile (node:internal/modules/cjs/loader:1065:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at [eval]:1:13
    at Script.runInThisContext (node:vm:129:12)

It's not difficult to be Node.js compatible. You either write CommonJS modules or EcmaScript modules (ESM). To use ESM you need to follow the Node.js documentation here: https://nodejs.org/api/esm.html
Basically Node.js requires to flag files either as CommonJS (which is the default) or as ESM (with "type": "module" or .mjs).
In Node.js ESM you allow need to have fully specified requests. So instead of ./.internal/core/Root you need ./.internal/core/Root.js.

Build tools are usually less restrictive, as they want to be backward-compatible to ESM code that was written before the spec has been fully completed. Node.js follows the spec more strictly, similar to browsers where similar restrictions apply (type="module" on script tag and fully specified requests in imports).

I don't see anything in that blog about "type": "module" (which has never been needed, and is still not needed by any build systems).

I can see that here: https://nextjs.org/blog/next-11-1#es-modules-support
but maybe @balazsorban44 added that in the meantime (EDIT: he did not). More importantly it links to the Node.js ESM documentation which is the source you should look at for Node.js ESM info.

Using "type": "module" is not standard, it is Node.js only, and it is not used by any JS build systems.

Yep that notation origins from Node.js, but at least webpack also supports it to flag modules as ESM. Probably other tools too.

Note that the HTML spec has something similar with type="module" at <script> tags or type: "module" for new Worker.

As I said, if you are requiring "type": "module" you will break many thousands of ES6 packages which work correctly with every other build system.

Next.js never supported ESM-only packages without "type": "module". Before Next.js 12 it always used the CommonJS version of a package and didn't support ESM-only packages at all. Since Next.js 12 it will use an ESM version of a package if available (and flagged with "type": "module" or .mjs) and that will indeed break if you provide a broken ESM version (flagged with "type": "module", but e. g. not using fully specified requests; that will also break in webpack 5).

This has nothing to do with ES6 vs CommonJS, our code uses 100% standard ES6 modules and has worked for years in every other build system. This is a Next.js specific issue.

100% standard ES6 modules is actually true if you are only referring to the EcmaScript Modules specificiation, which afaik only specifies syntax and semantic and leaves it unspecified which sources code is Script and which Module, and how requests are resolved.

If you look at ESM in browsers (not sure which spec, probably HTML integration and Web APIs), they actually require to specified which resources are scripts and which are modules. And they also require file extensions.


TL'DR: Next.js doesn't bundle node_modules before execution them on server side. They must be executable in Node.js. For ESM follow the Node.js ESM documentation.


next-transpile-modules will enable bundling for (some) node_modules, which pipes them through webpack. webpack ESM is less restrictive compared Node.js ESM, so that works around the issue. But bundling has a performance cost, and not all packages are bundle-able.

@Pauan
Copy link

Pauan commented Dec 13, 2021

@sokra Next.js (in all versions) uses Node.js to execute code for SSR or in API routes. This means any code you import from node_modules need to be compatible with Node.js.

Our code will never work in Node, because (like many thousands of other npm packages) it uses browser APIs like the DOM. It is a browser-only library.

Next.js is a React framework, it is expected that most of the npm dependencies will be browser-only.

Yep that notation origins from Node.js, but at least webpack also supports it to flag modules as ESM. Probably other tools too.

As you are aware, Webpack works perfectly fine without "type": "module" (though there are some changes if you choose to use it).

Note that the HTML spec has something similar with type="module" at <script> tags or type: "module" for new Worker.

That is up to the user, but package.json is for the library authors, not the users.

if you provide a broken ESM version (flagged with "type": "module", but e. g. not using fully specified requests; that will also break in webpack 5).

An ES6 module that does not use "type": "module" is not broken, it is perfectly valid and works fine in browsers (and all bundlers), and will continue to work in the future. It is even valid in Node, as long as it uses the .mjs extension (though that's a whole different story...)

If you look at ESM in browsers (not sure which spec, probably HTML integration and Web APIs), they actually require to specified which resources are scripts and which are modules. And they also require file extensions.

Yes, but they do not use "type": "module", they use a completely different system which is done by the user (not the library author), so it works with all libraries without any changes. I am very well aware of how ES6 modules work.

TL'DR: Next.js doesn't bundle node_modules before execution them on server side. They must be executable in Node.js. For ESM follow the Node.js ESM documentation.

Perhaps that makes sense for SSR, but if you require that for all npm packages (including browser-only npm packages) then you will break and fragment the entire ecosystem, because the vast majority of packages do not use "type": "module", and because they will never be run in Node, it is a bit silly to require them to add it.

And if you go even further and require strict Node.js compatibility (such as mandating a file extension) then you will break even more code, for example all TypeScript projects (because TypeScript has quite bad handling for file extensions). This will require massive changes to an incredible number of packages, despite those packages working perfectly fine in Webpack.

In many cases it's not even possible to fix the problem, because the library author is no longer maintaining the package. Or perhaps the problem is in a dependency of a dependency of a dependency of a dependency.

This "boil the ocean" strategy of trying to force everybody to change was tried before (with the .js vs .mjs debate), and it failed completely. There are too many packages and too many authors, this is not something that can be forced, it must be done gradually over time, with an incremental migration strategy.

@Pauan
Copy link

Pauan commented Dec 14, 2021

To be clear, migrating to "type": "module" is not trivial, this is what we would need to do to make our library work with Next.js:

  1. Add in "type": "module", this part is easy.

  2. Add in a .js extension to all of our file imports and also make other changes.

    This is not trivial, our library is over 118,000 lines of code.

  3. Convince all of our dependencies to also do step 1 and 2.

  4. Convince all of our dependencies of dependencies...

  5. Convince all of our dependencies of dependencies of dependencies...

This is a very large amount of work. None of our dependencies use "type": "module", and some of them aren't even maintained, this is outside of our control. That's the messy reality of the JS ecosystem. And if even one tiny transitive dependency doesn't support "type": "module" then the entire thing breaks.

Hypothetically, let's say we somehow manage to Nodeify our library and all our dependencies. If we add a new dependency, and that dependency isn't Nodeified, then it breaks all of our Next.js users. And if one of our dependencies adds a new transitive dependency which isn't Nodefied, it breaks all of our Next.js users.

So in practice Next.js users will need to transpile almost every npm package, because most npm packages do not use "type": "module" (or one of their transitive dependencies doesn't use "type": "module"). This is what I meant by "fragmenting the entire ecosystem". The documentation does not explain this severe limitation, and you will receive a lot of reports from users about this problem.

This is a horrible user experience, and a horrible library author experience. None of the other frameworks have this problem: React, Angular, Vue, Svelte, Snowpack, Webpack, Rollup, etc. etc. etc.

Here is one possible solution to this: when importing an npm package, check if it has a "type" field in package.json. If it does, then don't transpile it. If it doesn't, then transpile it. This fixes all of the problems:

  1. All existing packages Just Work without the user needing to manually transpile them.
  2. If a package has been Nodeified then it isn't transpiled, which improves compilation times.
  3. Library authors can incrementally Nodeify their package even if their dependencies haven't been Nodeified yet.
  4. Library authors can freely add new dependencies without worrying about if that dependency has been Nodeified or not.

Automatically transpiling packages is a far better experience compared to giving an error and forcing the user to manually transpile.

@sokra
Copy link
Member

sokra commented Dec 14, 2021

If you don't want to or can't provide a Node.js compatible version of the library that's fine. There is a way to opt-out of SSR for parts of a Next.js application via dynamic(() => import("..."), { ssr: false } (see https://nextjs.org/docs/advanced-features/dynamic-import#with-no-ssr).
This loads that part only on client-side. That's not the default, since Next.js is focused around optimal performance, which depends on SSR/SSG.

3. Convince all of our dependencies to also do step 1 and 2.

Maybe that wasn't clear from my first comment, but you can use ESM or CommonJS in Node.js. So there is no need to migrate all dependencies to ESM. They can also be in CommonJS. It's only a problem if they are only in ESM which is not Node.js compatible. Even if your library is ESM, your dependencies can be CommonJS.

@alienbuild
Copy link
Author

If you don't want to or can't provide a Node.js compatible version of the library that's fine. There is a way to opt-out of SSR for parts of a Next.js application via dynamic(() => import("..."), { ssr: false } (see https://nextjs.org/docs/advanced-features/dynamic-import#with-no-ssr). This loads that part only on client-side. That's not the default, since Next.js is focused around optimal performance, which depends on SSR/SSG.

  1. Convince all of our dependencies to also do step 1 and 2.

Maybe that wasn't clear from my first comment, but you can use ESM or CommonJS in Node.js. So there is no need to migrate all dependencies to ESM. They can also be in CommonJS. It's only a problem if they are only in ESM which is not Node.js compatible. Even if your library is ESM, your dependencies can be CommonJS.

Just to chime in on this the build still failed for me even when dynamically importing.

@sokra
Copy link
Member

sokra commented Dec 14, 2021

Just to chime in on this the build still failed for me even when dynamically importing.

I tried it. Works fine for me.

// pages/index.js

import dynamic from 'next/dynamic';
const Chart = dynamic(() => import('../components/Chart'), { ssr: false });

export default () => {
  return (
    <div>
      <Chart />
    </div>
  );
};

and for components/Chart.js I copied the full example from here: https://www.amcharts.com/docs/v5/getting-started/integrations/react/

@alienbuild
Copy link
Author

Just to chime in on this the build still failed for me even when dynamically importing.

I tried it. Works fine for me.

// pages/index.js

import dynamic from 'next/dynamic';
const Chart = dynamic(() => import('../components/Chart'), { ssr: false });

export default () => {
  return (
    <div>
      <Chart />
    </div>
  );
};

and for components/Chart.js I copied the full example from here: https://www.amcharts.com/docs/v5/getting-started/integrations/react/

I did try that initially and it was failing with the original error posted here, but tried again and seems to be working now, thank you

@Pauan
Copy link

Pauan commented Dec 15, 2021

@sokra Maybe that wasn't clear from my first comment, but you can use ESM or CommonJS in Node.js. So there is no need to migrate all dependencies to ESM.

You are misunderstanding the situation. This was never about ES6 vs CommonJS. All of our dependencies use ES6 modules. Our own code uses ES6 modules. We have used ES6 modules for several years. We have many years of experience with many build systems, because our customers use our library in many different situations.

But because we don't use "type": "module" our Next.js customers cannot use our library, because Next.js gives errors. This is about 100% valid ES6 libraries being broken in Next.js. This is about fragmenting the ecosystem, because the vast majority of ES6 libraries on npm do not use "type": "module".

ES6 modules are very common on npm (I have personally seen far more ES6 libraries on npm compared to CommonJS libraries), and yet they do not work in Next.js (even though they work in every other build system).

If Next.js is choosing to break 99+% of ES6 libraries on npm, that is something that really needs to be explained clearly in the documentation, or else you will get many complaints from users. Right now the documentation makes it seem like ES6 modules should work, even though they don't.

There is a way to opt-out of SSR for parts of a Next.js application

That is good to know, we will inform our customers about that.

@sokra
Copy link
Member

sokra commented Dec 16, 2021

because the vast majority of ES6 libraries on npm do not use "type": "module".

If Next.js is choosing to break 99+% of ES6 libraries on npm

Actually that made me curious what the real numbers are here. So I wrote a script to run over the top 1000 most depended on packages with the following results: (The script isn't perfect, and might have a few false postive/negatives)

  • 7.1% with type: "module"
  • 0.8% with type: "commonjs"
  • 0.0% with main: "*.mjs"
  • 1.0% with main: "*.cjs"
  • 4.6% with no main
  • 4.6% contains ESM code in the file referenced by main
  • 90.8% contains non ESM code in the file referenced by main
  • 14.4% with module: "..." (non-standard)
  • 4.5% with browser alternative main (non-standard)
  • 11.7% have an exports field
  • 3.7% have a .mjs file referenced by the exports field
  • 1.1% have a .cjs file referenced by the exports field
  • 8.3% have multiple files references by the exports field
  • 10.1% contains ESM code in a file referenced by the exports field
  • 8.1% contains non ESM code in a file referenced by the exports field
  • 5.6% contains a browser field with internal aliasing
  • 8.8% have the sideEffects field

So the vast majority are CommonJs libraries. Only 110 packages contain ESM at all. For these 110 packages 65 of them also provide a CommonJS alternative. So only 45 packages are "only ESM". From that 45 packages 39 have indeed a type: "module" in the top-level package.json. That leaves us with 6 potential packages that might be problematic:

  • @polymer/polymer: That's an ESM package that's not Node.js compatible. You would only be able to use that client-side only. Related to Custom Elements in HTML, so that won't work in Node.js.
  • vue-loader: Not something you would use in next.js (and it's actually a false positive, since it doesn't really contain ESM code)
  • react-native: Not something you would use in next.js
  • rollup-plugin-commonjs: Not something you would use in next.js (and it's actually a false positive, since it doesn't really contain ESM code)
  • @types/uuid: Typings
  • @jupyterlab/application: That's an ESM package that's not Node.js compatible.

So there is only a single packages that will break @jupyterlab/application for these list of packages.

@Pauan
Copy link

Pauan commented Dec 16, 2021

So I wrote a script to run over the top 1000 most depended on packages with the following results

I think you should run the script over the top 1000 most depended upon browser libraries. Next.js is a React framework, it is expected that most of the bundled dependencies would be browser-based libraries (such as React libraries). Most of the things you mentioned are build tools that are run at compile time, not code which is bundled with the application.

You will need to exclude things which are put into devDependencies (like webpack) because they aren't bundled with the application. Of course devDependencies will be primarily CommonJS, because they're Node programs that are intended to be run in Node. But those are not the sorts of things which are being bundled by Next.js

That's why I was speaking from my personal experience working on many npm projects, because I was taking into account which dependencies are actually being bundled with the package (and not just being run during compile time, like CLI tools). Those are the packages which actually cause build errors in Next.js, not the compile-time tools.

So perhaps the most fair comparison in this situation would be to look at the top 100+ React libraries. That is extremely relevant for Next.js, because it is a React framework. And those libraries are usually bundled with the application.

@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

5 participants