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

'import' and 'export' may only appear at the top level #304

Closed
maneetgoyal opened this issue Apr 12, 2020 · 28 comments
Closed

'import' and 'export' may only appear at the top level #304

maneetgoyal opened this issue Apr 12, 2020 · 28 comments

Comments

@maneetgoyal
Copy link

  • Rollup Plugin Name:
  • Rollup Plugin Version: @rollup/plugin-commonjs
  • Rollup Version: 11.1.0
  • Operating System (or Browser): Mac OS
  • Node Version: 12.13.1

How Do We Reproduce?

Expected Behavior

Bundling worked with lodash-es without any issues till v. 11.0.2.

Actual Behavior

!] Error: 'import' and 'export' may only appear at the top level
abc: ../../node_modules/lodash-es/isBuffer.js (1:0)
abc: 1: import root from './_root.js';
abc:    ^
abc: 2: import stubFalse from './stubFalse.js';
abc: Error: 'import' and 'export' may only appear at the top level
abc:     at error (/xxx/node_modules/rollup/dist/shared/rollup.js:10399:30)
abc:     at Module.error (/xxx/node_modules/rollup/dist/shared/rollup.js:14789:16)
abc:     at tryParse (/xxx/node_modules/rollup/dist/shared/rollup.js:14682:23)
abc:     at Module.setSource (/xxx/node_modules/rollup/dist/shared/rollup.js:15080:30)
abc:     at /xxx/node_modules/rollup/dist/shared/rollup.js:17086:20
abc:     at runNextTicks (internal/process/task_queues.js:58:5)
abc:     at processImmediate (internal/timers.js:412:9)
abc:     at async Promise.all (index 114)
abc:     at async Promise.all (index 5)
abc:     at async Promise.all (index 0)
abc: error Command failed with exit code 1.
@ceigey

This comment has been minimized.

@mattpilott

This comment has been minimized.

@shellscape
Copy link
Collaborator

Thanks for opening an issue. Citing the issue template:

Issues without minimal reproductions will be closed! Please provide one by:

  1. Using the REPL.it plugin reproduction template at https://repl.it/@rollup/rollup-plugin-repro
  2. Provide a minimal repository link (Read https://git.io/fNzHA for instructions).
    These may take more time to triage than the other options.
  3. Using the REPL at https://rollupjs.org/repl/

Please add a reproduction and we'll be happy to triage further.

Please refrain from +1 replies. They'll be removed.

@CadenP
Copy link

CadenP commented Apr 12, 2020

Here's a reproduction with the specific package (not a direct dependency) that's causing me issues:
https://repl.it/repls/PoliticalTestyMegahertz

Note that @rollup/[email protected] works, but @rollup/[email protected] fails.

@shellscape
Copy link
Collaborator

Thanks for adding that repro.

@Conduitry
Copy link
Contributor

There's another helpful-sounding comment here that tries to dig a little more into what's happening.

@tiefenb

This comment has been minimized.

@shellscape
Copy link
Collaborator

@tiefenb Please don't add +1 comments to issues in this repo. If you have a reproduction to add, please do share it but use one of the methods mentioned above.

@eswat2
Copy link

eswat2 commented Apr 14, 2020

Rollup Plugin Name: @rollup/plugin-commonjs
Rollup Plugin Version: 11.1.0
Rollup Version: 2.6.1
Operating System: Mac OS (10.15.4)
Browser: Chrome (83.0.4103.7)
Node Version: 12.16.1

i have a small Svelte demo that i keep up to date with the latest toolchains and i found that it fails to build when i updated @rollup/plugin-commonjs from "11.0.2" to "11.1.0" this weekend.

works -- https://github.com/eswat2/s4e-autos
broken -- https://github.com/eswat2/s4e-autos/tree/broken-build-compile-error-import-export

The only difference between these 2 is the version of @rollup/plugin-commonjs in package.json.

Running yarn build in master for this repo, you see what you'd expect:

➜  s4e-autos git:(master) yarn build
yarn run v1.22.4
warning ../package.json: No license field
$ rollup -c

src/main.js → public/build/bundle.js...
created public/build/bundle.js in 10.6s
✨  Done in 11.15s.

Now, running yarn build in the branch gives this:

➜  s4e-autos git:(broken-build-compile-error-import-export) yarn build
yarn run v1.22.4
warning ../package.json: No license field
$ rollup -c

src/main.js → public/build/bundle.js...
[!] Error: 'import' and 'export' may only appear at the top level
node_modules/symbol-observable/es/index.js (2:0)
1: /* global window */
2: import ponyfill from './ponyfill.js';
   ^
3:
4: var root;
Error: 'import' and 'export' may only appear at the top level
    at error (/Users/richardhess/github/zeit/s4e-autos/node_modules/rollup/dist/shared/rollup.js:10399:30)
    at Module.error (/Users/richardhess/github/zeit/s4e-autos/node_modules/rollup/dist/shared/rollup.js:14789:16)
    at tryParse (/Users/richardhess/github/zeit/s4e-autos/node_modules/rollup/dist/shared/rollup.js:14682:23)
    at Module.setSource (/Users/richardhess/github/zeit/s4e-autos/node_modules/rollup/dist/shared/rollup.js:15080:30)
    at /Users/richardhess/github/zeit/s4e-autos/node_modules/rollup/dist/shared/rollup.js:17086:20
    at async Promise.all (index 3)
    at async Promise.all (index 1)
    at async Promise.all (index 1)
    at async Promise.all (index 1)
    at async Promise.all (index 0)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I hope this example helps you resolve the issue...

Richard

UPDATE: 2020.04.14

i refactored master to use svql instead of svelte-apollo and after i got it all working i upgraded to @rollup/plugin-commonjs 11.1.0 and it all works like a charm. The branch is still broken if i try to upgrade, but master is working fine with all the latest npm packages.

Not sure if this helps or hurts your attempt to solve the problem, but it's another insight into the problem...

@eswat2
Copy link

eswat2 commented Apr 17, 2020

seems like ponyfill.js is one of the common elements in a few of the issues reported/linked here:

node_modules/symbol-observable/es/index.js

import ponyfill from './ponyfill.js';

Looking at the code behind that you find this:

export default function symbolObservablePonyfill(root) {
	var result;
	var Symbol = root.Symbol;

	if (typeof Symbol === 'function') {
		if (Symbol.observable) {
			result = Symbol.observable;
		} else {
			result = Symbol('observable');
			Symbol.observable = result;
		}
	} else {
		result = '@@observable';
	}

	return result;
};

and that was last updated 4 years ago specifically to support rollup:

benlesh/symbol-observable@7a41de9

https://github.com/benlesh/symbol-observable/blob/master/es/ponyfill.js

Maybe it's time for a rewrite?...

@LarsDenBakker
Copy link
Contributor

I ran into this issue as well, I did some debugging and I found that this file is incorrectly detected as a commonjs module: https://unpkg.com/[email protected]/es/index.js?module

It's possible this is because it checks for a module global. commonjs plugin 11.1.0 brings dynamic imports support, which changed quite a lot of things in the plugin.

@spectras
Copy link

spectras commented Apr 17, 2020

I hit the same issue. For me it's React that is not detected correctly. Which is interesting because React's index.js is like this:

'use strict';

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react.production.min.js');
} else {
  module.exports = require('./cjs/react.development.js');
}

Unfortunately I could not dig further at the moment, but it looks like when upgrading from 11.0.2 to 11.1.0, this now gets interpreted as imports within a if. Triggering the same error message as OP's.

@arantes555
Copy link

For me the error log is a bit different:

SyntaxError: /Users/XXXX/node_modules/@sentry/utils/esm/misc.js: 'import' and 'export' may only appear at the top level (4:0)

  2 | 
  3 | var misc = commonjsHelpers.createCommonjsModule(function (module) {
> 4 | import { isString } from './is';
    | ^
  5 | import { snipLine } from './string';
  6 | /**
  7 |  * Requires a module which is protected against bundler minification.
    at Parser.raise (/Users/XXXX/node_modules/@babel/parser/lib/index.js:6325:17)

But when I do a head ./node_modules/@sentry/utils/esm/misc.js, it starts with:

import { isString } from './is';
import { snipLine } from './string';

Without the commonjsHelpers.createCommonjsModule thing.

So it looks like the file already passed through the commonJS plugin somehow, and is passing again ?

@shellscape
Copy link
Collaborator

shellscape commented Apr 17, 2020

Notice to Future Commenters

Please do not reply with "me too" or "same here" or "+1" replies unless you are providing specifics on the code in the commonjs plugin that can be used for triage. We have enough of a sample of what isn't working at this point.

If you wish to provide helpful triage information that includes references to the code in the commonjs plugin here in this repository, please do share. If you want to provide an additional reproduction, please do so by:

  1. Using the REPL at https://rollupjs.org/repl/, or
  2. Using the REPL.it reproduction template at https://repl.it/@rollup/rollup-repro
    (allows full use of all rollup options and plugins), or
  3. Provide a minimal repository link (Read https://git.io/fNzHA for instructions).
    These may take more time to triage than the other options.

If you don't heed this request, your reply may be deleted. We are asking this to keep the noise down on this issue.

@FredKSchott
Copy link
Contributor

Like others I encountered this error through node_modules/symbol-observable/es/index.js. If it helps to find out what's wrong, I noticed that changing that file name index.js -> index.mjs seemed to solve the problem for that specific file.

@tommedema
Copy link

This was resolved for me by downgrading @rollup/plugin-commonjs from 11.1.0 to 11.0.2, though this did introduce a warning that it's expecting a peer dependency of rollup@^1.20.0

@shellscape
Copy link
Collaborator

@lukastaegert can you provide any guidance on this one?

clarkdo added a commit to nuxt/nuxt that referenced this issue Apr 26, 2020
the fixed package resolution can be removed after rollup/plugins#304 gets fixed
clarkdo added a commit to nuxt/nuxt that referenced this issue Apr 26, 2020
@mikeifomin
Copy link

Temporary fixed with exclude option (https://github.com/rollup/plugins/tree/master/packages/commonjs#exclude)
in rollup.config.js

   commonjs({
       exclude: ['node_modules/symbol-observable/es/*.js'],
    }),

@manucorporat
Copy link

manucorporat commented Apr 29, 2020

Wondering if this is causing by the removal of this early exit:

cbc341d?diff=split#r38821925

manucorporat referenced this issue Apr 29, 2020
* Implemented support for dynamic requires (transferred PR)

Moved from rollup/rollup-plugin-commonjs#331

* Only add dynamic loader code when dynamic feature is enabled

* test(commonjs): update snapshots for easier diffing

* Automatically remove user paths

* test(commonjs): Prepare tests to support code-splitting

* test(commonjs): Try to add a code-splitting test

* Fixed code-splitting support

* Cleanup: avoid importing commonjs-proxy when we only need to register

* Fixed test

* Updated pnpm-lock

* Updated snapshots

* Satisfy linter

Co-authored-by: Lukas Taegert-Atkinson <[email protected]>
@ivan-nemtinov
Copy link

The simplest solution for now: import correct index file directly instead of letting rollup to determine module type:
import $$observable from 'symbol-observable/index'

@FredKSchott
Copy link
Contributor

This isn't an import that I've written myself / that I can control, it's one inside of my node_modules directory.

Just in case my original comment downplayed the severity, this is a fairly serious bug. Snowpack is stuck on "~11.0.0" until this is resolved.

@danielgindi
Copy link
Contributor

It seems like detection of whether to transform the module or not should be a bit more complicated.
What happens is that it detects usage of module due to the } else if (typeof module !== 'undefined') { which is meant for detecting node vs browser envs.
But it's only sniffing around, not using any require call from module.
I'm working on a fix to take this into account.

danielgindi added a commit to danielgindi/rollup-official-plugins that referenced this issue Apr 30, 2020
@danielgindi
Copy link
Contributor

There's a fix pending, with the test replicating the repl supplied by @CadenP.

In the future, please try to keep repros localized, without external dependencies. This helps in detection of the pain point.

@CadenP
Copy link

CadenP commented Apr 30, 2020

There's a fix pending

Great news! Thanks for all of your work!

In the future, please try to keep repros localized, without external dependencies. This helps in detection of the pain point.

Makes sense, I'll try to do that.

@tobius
Copy link

tobius commented Apr 30, 2020

What version will this be available in?

@shellscape
Copy link
Collaborator

you went and did it

jeremychone added a commit to BriteSnow/node-vdev that referenced this issue May 2, 2020
- rollup - Downgrade @rollup/plugin-commonjs from 11.1.0 to 11.0.0 to avoid issue rollup/plugins#304 on moment
- rollup - Silent cyclic rollup warnings for module moment.
@aminya

This comment has been minimized.

@jonataswalker

This comment has been minimized.

@rollup rollup deleted a comment from aminya May 7, 2020
@rollup rollup locked as resolved and limited conversation to collaborators May 7, 2020
mihilranathunga added a commit to mihilranathunga/microbundle that referenced this issue May 22, 2020
LarsDenBakker pushed a commit to LarsDenBakker/plugins that referenced this issue Sep 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.