plugins: CommonJS plugin breaks isEntry for entry points

Rollup Plugin Name: @rollup/plugin-commonjs
Rollup Plugin Version: 22.0.0
Rollup Version: 2.70.2
Operating System (or Browser): Linux 64-bit NixOS
Node Version: v16.14.0
Link to reproduction (⚠️ read below): https://replit.com/@Pauan1/rollup-plugin-repro#rollup.config.js

Expected Behavior

The console should contain the following:

/home/runner/rollup-plugin-repro/input.js true

Actual Behavior

The console contains the following:

/home/runner/rollup-plugin-repro/input.js false
/home/runner/rollup-plugin-repro/input.js?commonjs-entry true

This means that the entry point is returning isEntry: false even though it should return isEntry: true

Additional Information

I noticed this bug while using the Rust Rollup plugin. With that plugin, you can import Cargo.toml files:

export default {
    input: {
        foo: "Cargo.toml",
    }
};

The Rust plugin requires isEntry in order to generate correct code.

However, even though those Cargo.toml files are not CommonJS (they’re not even JS!) the commonjs plugin breaks them by causing isEntry to return false even though it should return true.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 22 (8 by maintainers)

Most upvoted comments

Hi guys, I hit same error:

  0:0  error  Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: index.ts?commonjs-entry.
The extension for the file (.ts?commonjs-entry) is non-standard. You should add "parserOptions.extraFileExtensions" to your config

solved by downgrading @rollup/plugin-commonjs to version 21 🤷 🙂

luckylooke on Jun 23, 2022

@lukastaegert Sorry for the delay. Although that PR does fix the immediate issue, I have an idea:

Right now the resolve method returns an object with various information. What if Rollup added a new property to that? So the resolve method would return something like this:

{
    id: string,
    external: boolean | "absolute",
    moduleSideEffects: boolean | 'no-treeshake',
    syntheticNamedExports: boolean | string,
    meta: {[plugin: string]: any},
    resolvedBy: string | null,
}

The new resolvedBy property would be the name of the plugin which resolved it. If all of the plugins return null then resolvedBy will be null.

So instead of checking the id, the node-resolve plugin would check the resolvedBy property: if it’s null then it knows that the other plugins didn’t resolve it.

The advantage is that this will work even if a plugin doesn’t change the id, but instead changes other settings (like external, or meta, or moduleSideEffects).

This is essentially the same as enforce: "post", since it means that node-resolve will only resolve the ids if no other plugin resolves it first.

Pauan on May 11, 2022

While the “hack” would give you a way to solve the issue independently and quickly (and I do not think it is really terrible: Sometimes you just really want a hook to tun first or last and need to override ordering by users). Still, here is my proposal for a long-term solution in node-resolve:

#1181

If you can verify that this solves the issues, that would help in getting it merged.

lukastaegert on May 1, 2022

#1180 will avoid proxying for non-cjs entry points, I think that will make everyone happier

lukastaegert on Apr 30, 2022