Exempt Resolver/polyfills/require.js from extract-dependencies

[remyrylan]

Summary

metro-bundler/src/JSTransformer/worker/extract-dependencies.js parses every file to gather the usage of require(...) and extract the module name for building the dependency array.

The problem is that exract-depdendencies.js additionally analyzes metro-bundler/src/Resolver/polyfills/require.js which includes require(moduleId). So it's failing every single time because moduleId is an Identifier (variable) instead of a StringLiteral as required in extract-dependencies.js.

Test plan

Generate a successful build as normal.

[remyrylan]

The CI check is failing on code unrelated to my PR.

[cpojer]

This is weird, and hasn't been a problem for us. I would prefer not to hardcode the specific module path here. cc @davidaurelio what do you think?

[remyrylan]

@cpojer I don't know if perhaps I'm hitting the issue because I use a custom rn-cli.config.js just to use a custom transform for TypeScript, but I hit the issue literally every single time. My build gets to 99%+ and then I get the red screen with "require() must have a single string literal argument". I edited extract-dependencies.js to inspect the value of nodeArgs in the pushDependency function and it was always an Identifier (instead of StringLiteral with the name moduleId.

So then I hoped over src/JSTransformer/worker/index.js and wrapped a try/catch around the use of extractDependencies to catch and log the filename of the problematic file... and it prints out src/Resolver/polyfills/require.js every time.

To work around the issue, I implemented a node module resolver hook to intercept the import of src/JSTransformer/worker/index.js and replace it with the code as seen here in my PR. Works perfectly fine and exactly as expected.

[cpojer]

How about instead ignoring that file from having deps extracted? It's almost certainly the config you have, and I apologize that the current config system allows you to shoot yourself in the foot.

[remyrylan]

@cpojer you mean add it to the blacklistRE in rn-cli.config.js? Trying it now.

[remyrylan]

@cpojer if I add the following in my rn-cli.config.js I still get bundling failed: Error: require() must have a single string literal argument.

  getBlacklistRE(platform) {
    return blacklist([
      /.*\/polyfills\/require\.js.*/
    ])
  },

Is this what you meant for me to try or is there something else?

[cpojer]

Is that on the same file? That would be weird.

[remyrylan]

@cpojer yup, same file as before -- always erroring on the require polyfill. Here's the contents of my rn-cli.config.js:

const blacklist= require('metro-bundler/src/blacklist')

export default {
  getBlacklistRE(platform) {
    return blacklist([
      /.*\/polyfills\/require\.js.*/
    ])
  },

  getTransformModulePath() {
    return require.resolve('./transformer.js')
  }
}

The transformer is very simple, just a basic TypeScript transform.

[cpojer]

Alright, this has devolved into a question thread rather than a PR. I don't think there is anything wrong with Metro, and I'd like to keep this code outside of the extraction code. For TypeScript compilation with RN, you could reach out to @orta. Thanks for understanding.

[remyrylan]

@cpojer This isn't a question thread. I'm providing you with the details of the scope of my setup so that there is less need for back-and-forth questions to resolve the problem.

The issue is completely unrelated to TypeScript, none of the problem is happening in the transform at all. Somehow even with adding polyfills/require.js (part of the Metro source code) to the blacklist, Metro still tries to extract dependencies from a file where it shouldn't be... A file completely outside of my code and never referenced in my code.

I am having no problem at all with TypeScript and in fact the previous version of Metro and React Native have worked perfectly fine in my setup for a long, long time. This is a regression issue in the latest version of Metro.

I understand that you're swamped and don't have much time to invest in issues, but I think it was premature to close this -- as I feel like this is an issue others will certainly experience. It's worth discovering a proper solution and at least updating documentation or something because I won't be the only person running into this. I will gladly contribute any updates needed to code or docs if we all get on the same page.

[cpojer]

Mind looking at the default config to figure out what's different with your config?

[remyrylan]

@cpojer - my sincere apologies, I was wrong and it is related to the custom transform in my rn-cli.config.js. Others may run into this problem, so if you think it's worthy of me documenting and updating docs, please let me know....

Basically, in my custom transform, I have a custom method that checks the file path to see if my transformer should actually do anything or just return the source code that was provided to it unchanged. Essentially, if a filename contains node_modules and then isn't followed after that by something like react|react-*|native-* then my transformer skips it and returns

return {
  filename,
  ast: null,
  code: src, // This is the src that is passed into the custom transform defined in rn-cli.config.js
  map: null
}

What is going on is that in previous versions of Metro and React Native, it was fine for me to just return the source of any Metro Bundler file unchanged -- that is now no longer the case. Metro Bundler must be added to my transform whitelist in order to accommodate the following two files that are added to your app bundle:

1. metro-bundler/src/Resolver/polyfills/prelude_dev.js
2. metro-bundler/src/Resolver/polyfills/require.js

My custom transform handles TypeScript when the file extension matches .ts or .tsx, otherwise it passes it off to Babel using the babel-preset-react-native/configs/main or babel-preset-react-native/configs/hmr.... So basically, in this case my transformer was never allowing either of those Metro files to hit Babel, and it looks like they need to now.

Everything works properly if I just allow any Metro Bundler files to pass through and get processed by Babel instead of doing the early return of the source as I posted above.

Also, @cpojer: thank you very much for taking the time to look into this. I know you guys are crazy busy.

[cpojer]

Hey, thanks for following up @jrylan. It's ok to be wrong sometimes, I am wrong often too :) I appreciate that you documented this issue so thoroughly here. I'm wondering if we should just whitelist these two files and transform them with a default worker in Metro to make this easier? That way your transformer doesn't have to concern itself with Metro's own code.