danielgindi/rollup-plugin-natives

Fails for `spellchecker` npm module

astoilkov opened this issue · 10 comments

I'm building an electron application using Rollup. I'm using this module to include native modules in the bundle.

rollup-plugin-natives successfully bundles chokidar (fsevents.node) but fails with spellchecker:

[commonjs--resolver] Unexpected character '�' (Note that you need plugins to import files that are not JavaScript)

loc: {
    column: 0,
    file: 'electron-boilerplate/node_modules/spellchecker/build/Release/spellchecker.node',
    line: 1
  },

pluginCode: 'PARSE_ERROR',
  plugin: 'commonjs--resolver',
  hook: 'resolveId',

My configuration is:

rollupPluginNatives({
    copyTo: join(baseViteConfig.build!.outDir!, 'node'),
})

I don't have much experience with Rollup. Sorry if I'm missing something obvious.

Please write down your whole configuration (other plugins too)

No other plugins. I'm running it through Vite.

const nativePlugin = rollupPluginNatives({
    copyTo: join(process.cwd(), './dist/vite/node'),
})
const specificViteConfig: UserConfig = {
    assetsInclude: ['**/*.bin'],

    plugins: [nativePlugin],

    build: {
        assetsDir: '.',
        emptyOutDir: false,
        outDir: join(process.cwd(), './dist/vite'),
        sourcemap: true,
    },
}

I can also make a small reproducible repo and send it to you?

That would be fine, as I don't know what vite configures for rollup.

Here is a reproducible example: https://github.com/astoilkov/rollup-plugin-natives-spellchecker-issue.

Run yarn run build to observe the problem.

You can comment on the spellchecker section and uncomment the chokidar section and you will see that chokidar builds successfully.

Thanks for spending time on this! Let me know if you need anything else.

@danielgindi If you give me some guidance, I can also try to debug this.

Sorry I was preoccupied. What we need to see is why rollup try to compile .node files. It should be excluded. Maybe you could specify exclusions in the config file.
I'll take a look soon.

Thanks to your comment I figured it out. If I change the configuration like this it works:

plugins: [
  {
    enforce: "pre",
    ...rollupPluginNatives({
      copyTo: "node-natives",
    }),
  },
],

The important thing is enforce: "pre".

I think this can be added to the return value of the plugin.

@astoilkov Well this is not a rollup setting, so it's a good reason not to add it to the plugin's output. I can't know how it will interfere if an enforce setting is added to rollup in the future...

But you can:

  1. Send a PR to update the current README so Vite users will know about it
  2. Send a PR to Vite (https://github.com/vitejs/vite/blob/main/docs/guide/using-plugins.md, https://vite-rollup-plugins.patak.dev/) as there seems to be a maintained list of plugins with their respective enforce configuration.

Btw I see that even the popular eslint plugin is on the compatibility list with "enforce: pre" required. It must be something about the way that Vite manages the order of plugins, as you are not actually configuring the whole rollup chain yourself.

Yeah, it puts built-in plugins first by default. Thanks.