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

[@rollup/plugin-esm-shim] Shim inserted incorrectly if source file has import in comment #1709

Open
EvanHahn opened this issue Apr 18, 2024 · 1 comment

Comments

@EvanHahn
Copy link

Expected Behavior

The shim should be inserted in the right place, even if the source file contains static imports in comments.

For example, the shim should be inserted right after the import in the file below:

import foo from "./other_module.js";

// Triggers the shim
require();

/*
import BAD from "./bad";
*/

Actual Behavior

The shim is inserted in a strange place, causing syntax errors.

Additional Information

I believe this bug happens because the plugin tries to insert the shim after the last import, but incorrectly detects them when they're inside comments.

imports are detected with the following regular expression:

export const ESMStaticImportRegex =
/(?<=\s|^|;)import\s*([\s"']*(?<imports>[\w\t\n\r $*,/{}]+)from\s*)?["']\s*(?<specifier>(?<="\s*)[^"\n]*[^\s"](?=\s*")|(?<='\s*)[^'\n]*[^\s'](?=\s*'))\s*["'][\s;]*/gm;

This expression is unaware of comments.

I would probably solve this by parsing the AST rather than using a regular expression. Let me know if you'd like me to make that pull request.

@Edgar-P-yan
Copy link

Edgar-P-yan commented May 15, 2024

Hi! I hit the same bug while was working on my library that needs to be compiled to both esm and cjs modules (https://github.com/Edgar-P-yan/node-systray-v2/). The shims are inserted in the middle of other import statements causing syntax errors. But i didn't have any commented import keywords. So probably the ESMStaticImportRegex has other bugs in it too. The version i'm using is the latest at this moment (v0.1.6). Here is the result:

import * as child from 'child_process';
import * as readline from 'readline';
import * as path from 'path';
import * as os from 'os';
import * // <-- The import statement is cut in the middle
// -- Shims --
import cjsUrl from 'node:url';
import cjsPath from 'node:path';
import cjsModule from 'node:module';
const __filename = cjsUrl.fileURLToPath(import.meta.url);
const __dirname = cjsPath.dirname(__filename);
const require = cjsModule.createRequire(import.meta.url);
 as fs from 'fs-extra'; // <-- The unfinished import continues after the inserted shims
import { EventEmitter } from 'events';
import xdebug from 'debug';

So i wanted to share a simple and temporary solution to this, a slightly modified version of this plugin. This will also not work for commented import statements, but at least the regex is simpler, so you can adjust it. See the comments for more info about patches:

/**
 * An alternative to @rollup/plugin-esm-shim (https://github.com/rollup/plugins/tree/master/packages/esm-shim).
 *
 * The original ESM shim plugin has a bug: it inserts the shims
 * in wrong places causing syntax errors. This slightly modified
 * version of it is a very simple solution, which surely will not work
 * in every case, but at list does not brake specifically for my code.
 *
 * Whats different from the original?
 * The regex used to find import statements is changed to a more
 * simple one /^import\s.*';$/gm and the whole logic is a lot simpler,
 * so you can adjust the regex to make it work with your code, if it suddenly doesn't.
 */
function esmShimCustom() {
  const ESMShim = `
// -- Shims --
import cjsUrl from 'node:url';
import cjsPath from 'node:path';
import cjsModule from 'node:module';
const __filename = cjsUrl.fileURLToPath(import.meta.url);
const __dirname = cjsPath.dirname(__filename);
const require = cjsModule.createRequire(import.meta.url);
// -- End Shims --
`;
  const CJSyntaxRegex = /__filename|__dirname|require\(|require\.resolve\(/;

  return {
    name: 'esm-shim-custom',

    renderChunk(/** @type {string} */ code, _chunk, opts) {
      if (opts.format === 'es') {
        if (code.includes(ESMShim) || !CJSyntaxRegex.test(code)) {
          return null;
        }

        let endIndexOfLastImport = -1;

        // Find the last import statement and its ending index
        for (let match of code.matchAll(/^import\s.*';$/gm)) {
          if (match.length === 0 || typeof match.index !== 'number') {
            continue;
          }

          endIndexOfLastImport = match.index + match[0].length;
        }

        const s = new MagicString(code);
        s.appendRight(endIndexOfLastImport, ESMShim);

        const sourceMap = s.generateMap({
          includeContent: true,
        });

        /** @type {string[] | undefined} */
        let sourcesContent;
        if (Array.isArray(sourceMap.sourcesContent)) {
          sourcesContent = [];
          for (let i = 0; i < sourceMap.sourcesContent.length; i++) {
            const content = sourceMap.sourcesContent[i];
            if (typeof content === 'string') {
              sourcesContent.push(content);
            }
          }
        }

        return {
          code: s.toString(),
          map: {
            ...sourceMap,
            sourcesContent,
          },
        };
      }

      return null;
    },
  };
}

And the usage is straight-forward, just copy the function esmShimCustom provided above into your rollup.config.js, and add it to output.plugins:

const options = {
  output: [
    {
      banner,
      name: 'node-systray-v2',
      exports: 'named',
      sourcemap: true,
      file: './dist/index.mjs',
      format: 'esm',
      plugins: [esmShimCustom()], // <-- insert it right here
    },
    ...
  ],
  ...
}

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

No branches or pull requests

2 participants