fix: use custom Rollup plugin for import.meta.url

[mdjermanovic]

Refs eslint/eslint#15766, this should fix the issue.

dist/eslintrc.cjs diff (line 2383):

- const require$1 = Module.createRequire((typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __filename).href : (document.currentScript && document.currentScript.src || new URL('eslintrc.cjs', document.baseURI).href)));
+ const require$1 = Module.createRequire(require('url').pathToFileURL(__filename).toString());

The new version does not include code for browsers (typeof document ...), I'm not sure why Rollup's default transformation includes it when the output format is set to "cjs".

Without this change, the added test case fails with the same error as in eslint/eslint#15766.

[nzakas]

The change looks ok to me, but wondering:

1. wouldn’t it be better to file an issue with Rollup?
2. Will this break usage in browsers? (Linter references this package)

[fasttime]

Just a note: pathToFileURL returns a URL object, while import.meta.url is a string. This is not a problem as long as import.meta.url is only used for createRequire as it is here the case. But if we accept that, then we could also just return "__filename", because createRequire is perfectly fine with a path as argument.

[mdjermanovic]

You're right, import.meta.url should be transformed to a URL string. I have applied @aladdin-add's suggestion to fix this by using URL#toString(). Thanks!

[aladdin-add]

I've also noticed the issue when moving eslint to esm. there is a babel plugin doing the same thing - babel-plugin-transform-import-meta. wondering if we want to use that?

[mdjermanovic]

I've also noticed the issue when moving eslint to esm. there is a babel plugin doing the same thing - babel-plugin-transform-import-meta. wondering if we want to use that?

It's probably more robust than this text transformation, but I'm not sure if it warrants adding babel to the build? If there's a case where the babel plugin handles code better than this, I'd guess it would have to be a very edge case.

[fasttime]

@nzakas AFAICT Linter uses dist/eslintrc-universal.cjs, while this change would only affect the Node.js bundle dist/eslintrc.cjs, so if I'm not missing anything, at least for Linter, it should not make any difference. Reporting the issue to Rollup is a good idea and I was thinking to do that. For the time being, this PR still seems useful to fix the issue.

[snitin315]

LGTM

[mdjermanovic]

The change looks ok to me, but wondering:

1. wouldn’t it be better to file an issue with Rollup?

I've filed rollup/rollup#4461 now, waiting for response.

2. Will this break usage in browsers? (Linter references this package)

I believe not, as this particular module that uses import.meta.url (config-array-factory.js) is not used by Linter, neither directly nor indirectly. As @fasttime mentioned, Linter uses dist/eslintrc-universal.cjs bundle that doesn't include this code, and the custom transformation is applied only to dist/eslintrc.cjs

[nzakas]

Thanks @fasttime for all of your help on this.

@mdjermanovic up to you to either merge this or wait for a Rollup response.

[mdjermanovic]

All checks in eslint repo (eslint/eslint#15795) are passing with this branch.

Since there is still no response on rollup/rollup#4461, I'm going to merge this now. If and when Rollup fixes this issue, we can revert this change.