Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

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

Closed
marvinhagemeister opened this issue Aug 25, 2016 · 2 comments · Fixed by calvinmetcalf/rollup-plugin-node-globals#3

Comments

@marvinhagemeister
Copy link

I'm having trouble bundling handlebars. On bundling I get the following error:

Error parsing /myproject/node_modules/handlebars/dist/cjs/handlebars/no-conflict.js: 'import' and 'export' may only appear at the top level (4:0) in /myproject/node_modules/handlebars/dist/cjs/handlebars/no-conflict.js

My first try to fix this, was to use the named exports feature. But although I've explicitly specified handlebars as a named export, it seems to be ignored and I always get the same error message as above.

Here is my setup:

// rollup.config.js
const rollup = require('rollup');
const babel = require('rollup-plugin-babel');
const replace = require('rollup-plugin-replace');
const commonjs = require('rollup-plugin-commonjs');
const globals = require('rollup-plugin-node-globals');
const nodeResolve = require('rollup-plugin-node-resolve');

export default {
  entry: 'scripts/main.js',
  dest: 'public/assets/js/bundle.js',
  format: 'iife',
  moduleName: 'test',
  sourceMap: true,
  plugins: [
    globals(),
    nodeResolve({
      jsnext: true,
      main: true,
      browser: true,
      preferBuiltins: false
    }),
    commonjs({
      include: ['node_modules/**'],
      namedExports: {
        'node_modules/handlebars/dist/handlebars.min.js': ['handlebars']
      }
    }),
    babel({
      exclude: ['node_modules/**'],
      runtimeHelpers: true
    }),
    replace({
      'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV)
    })
  ]
}
// main.js
import Handlebars from 'handlebars';

console.log(Handlebars.compile('<h1>Hello World</h1>');
// .babelrc
{
  "presets": [
    ["es2015", {
      "modules": false
    }]
  ],
  "plugins": [
    "external-helpers",
    "transform-object-rest-spread"
  ]
}
@Rich-Harris
Copy link
Contributor

So, there are a few things going on here, it turns out. The error message you saw was caused by this plugin attempting to transform a module which had an import declaration added to it by rollup-plugin-node-globals. Moving that plugin to the bottom (so that the CommonJS module was converted to ES6 before the import got added) fixed that issue, but created another (which would be fixed by calvinmetcalf/rollup-plugin-node-globals#3. A workaround in the meantime is to ensure that rollup-plugin-node-globals comes before rollup-plugin-node-resolve in the plugins array).

After that, the bundle builds fine. But it doesn't run. Because path and fs are required from somewhere, they're treated as external dependencies, which means the IIFE expects them as arguments...

(function (fs, path) {
/* bundle goes here */
}(fs, path));

...but path and fs don't exist as variables on the page, so it blows up. Using rollup-plugin-node-builtins we can add a browser-friendly path (obviously only some of its functions work outside of Node), but there's no browser equivalent of fs.

We can use the globals option to replace those modules with whatever expression we like (since presumably path and fs are only there for some Node-specific functionality):

// rollup.config.js
export default {
  entry: 'scripts/main.js',
  dest: 'public/assets/js/bundle.js',
  ...,
  globals: { fs: '{}', path: '{}' }
};

That results in this:

(function (fs, path) {
/* bundle goes here */
}({}, {}));

But then we get this error...

bundle.js:3828 Uncaught ReferenceError: require is not defined

...because of this line inside the source-map module:

if (typeof define !== 'function') {
    var define = amdefine_1(module, require);
}

In case you weren't familiar with it, amdefine is a package that allows you to write AMD modules and have them converted into CommonJS modules at runtime by adding that snippet. It's exactly as ridiculous an idea as it sounds, and it means your bundle has to include this dead weight (in fact, now that I investigate, that's the only reason the bundle needs path – so that it doesn't need to implement dirname. Literally the only reason!). So we're attempting to transform a CommonJS module that is really an AMD module that only gets converted at runtime. Teetering atop this house of cards, it's a wonder that anything stable was ever built.

Of course, we don't actually need the source-map stuff, as evidenced by this block:

try {
  /* istanbul ignore next */
  if (typeof define !== 'function' || !define.amd) {
    // We don't support this in AMD environments. For these environments, we asusme that
    // they are running on the browser and thus have no need for the source-map library.
    let SourceMap = require('source-map');
    SourceNode = SourceMap.SourceNode;
  }
} catch (err) {
  /* NOP */
}

Rollup doesn't have the luxury of running the code to find out what's necessary and what isn't, so it has to import source-map. But we can disable it by modifying our config like so:

// rollup.config.js
plugins: [
  ...,
  nodeResolve({
    jsnext: true,
    main: true,
    browser: true,
    preferBuiltins: false,
    skip: [ 'source-map' ]
  })
],
external: [ 'source-map' ],

Then we hit another 'require is not defined' error caused by this code:

if ('function' !== 'undefined' && require.extensions) {
  require.extensions['.handlebars'] = extension;
  require.extensions['.hbs'] = extension;
}

Another runtime check, this time to see if we want to make it possible for Node users to compile templates at runtime by doing var compiled = require('template.hbs'). Which isn't even remotely a concern for browser users, but we still have to deal with it. (The 'function' !== 'undefined') thing is to allow UMD modules to work with this plugin.)

We can get rid of that whole block by replacing require.extensions with null:

// rollup.config.js
plugins: [
  ...,
  replace({
    'require.extensions': 'null'
  })
]

And after that, it works. Whether it will continue to work as you use more of Handlebars remains to be seen.

So the blame is shared between Rollup (for making it too easy for plugins to be in the wrong order) and the ghosts of module systems past. I wouldn't judge you for using Rollup to bundle your own code as a CommonJS module, then handing the result off to Browserify or Webpack to deal with the various layers of insanity that have accreted in the Node ecosystem over the past few years. There's also Rollupify, if you wanted to take a Browserify-first approach.

I've put a working version of the example in this repo, so you can poke and prod it: https://github.com/Rich-Harris/rollup-plugin-commonjs-issue-90

Thanks for coming on this wild adventure with me, hope it helps!

@marvinhagemeister
Copy link
Author

@Rich-Harris Wow, didn't know it was this bad with the dependencies! That is some quality investigation of yours! Really appreciate your detailed response.

Regarding the complexity of the bundling layers I'm totally on your side. In the past years people have found really crazy ways to make their libraries run both on node and inside the browser.

I agree that this is more the fault of library authors and nothing which rollup should invest time in supporting. I can't wait for the day when most modules are es6 based. Thanks for an awesome bundler! We've been using it with great success at work!

Thanks for the working repo, I can only imagine how much time it took to step through all these corner cases 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants