"require is not defined" after moving to ESM and rollup

[fniephaus]

We try and adopt template updates in our action from time to time and Convert to ESM and use rollup has caused an issue on Windows related to rollup/rollup#3434. Looking at the dist/index.js of this template, I also see a bunch of require calls. So this might also be an issue in this template.

I tried using transformMixedEsModules=true in here but that didn't seem to fix the problem.

Any help with this is appreciated!

Pinging @ncalteen because he's the author of #969.

[ncalteen]

I went down the rabbit hole on this one and think I found the issue. Most of the require statements are correctly handled with try/catch blocks and alternatives are provided. One example, below:

try {
  const crypto = require('node:crypto');
  random = (max) => crypto.randomInt(0, max);
} catch {
  random = (max) => Math.floor(Math.random(max));
}

The main issue I see is one of your project's dependencies, @actions/glob. When it is transpiled by rollup, the following is generated by a sub-dependency, minimatch.

var path = (function () { try { return require('path') } catch (e) {}}()) || {
  sep: '/'
};

Obviously, the fallback separator / isn't going to work on Windows. Doing some more digging, it looks like @actions/glob is still relying on a very old version of minimatch.

Testing with a simple clone of this repo, I set the following override re-packaged the action. The end result is no more erroneous path separators.

// package.json
{
  // ...
  "overrides": {
    "@actions/glob": {
      "minimatch": "^10.0.1"
    }
  }

I can't guarantee this will work 100% for you, but it should at least resolve the immediate error you're seeing. In the meantime, I opened an issue in the @actions/toolkit repo to request they update the version of minimatch being used.

[fniephaus]

Hi @ncalteen,
thanks a lot for looking into this and good to know that there is an easy way to fix it! We might want to wait until the minimatch dependency is updated in @actions/glob before rolling out another update to our users.

This is the first time I come across rollup and I'm actually surprised that it even outputs these try/catch blocks, given that they will always fail if they contain a require. Do you know of any option that emits what is in the catch block?

[ncalteen]

Have you tried setting ignoreTryCatch: false in @rollup/plugin-commonjs? I can't guarantee it'll work as I haven't tested it fully, but the output index.js no longer contains any require statements.

// See: https://rollupjs.org/introduction/

import commonjs from '@rollup/plugin-commonjs'
import nodeResolve from '@rollup/plugin-node-resolve'
import typescript from '@rollup/plugin-typescript'

const config = {
  input: 'src/index.ts',
  output: {
    esModule: true,
    file: 'dist/index.js',
    format: 'es',
    sourcemap: true
  },
  plugins: [
    typescript(),
    nodeResolve({ preferBuiltins: true }),
    commonjs({ ignoreTryCatch: false })
  ]
}

export default config

[ncalteen]

FYI, the reason I haven't included that in the template is because of the following:

Due to the conversion of require to a static import - the call is hoisted to the top of the file, outside of the try-catch clause.

Reference