Better support for imports with dynamic expressions

[eduardoboucas]

From my experience using esbuild, require/import calls that can't be statically analysed are the biggest cause of broken builds. It's understandable that esbuild can't resolve these expressions at build time, but I'd love to see us finding ways of iteratively addressing some of these cases, with the goal of reducing the likelihood of broken bundles. In my opinion, #1155 goes in the opposite direction.

As @evanw pointed out, these warnings were the only red flag telling users that they were generating a potentially broken bundle. Removing them favours a clean terminal over working applications.

Rather than re-introducing this warning, which people clearly aren't a fan of, I want to propose an alternative solution for surfacing these problems as a first step, but also to discuss mechanisms for solving them.

1. Add unresolved imports to metafile

Any imports that can't be statically resolved can be added to the metadata object, under an unresolvedImports array. For example, the following unresolvable import:

const locale = require(`./locale/${language}.json`)

... would generate the following entry in the metadata:

{
  "unresolvedImports": [
    {
      "expression": "`./locale/${language}.json`",
      "location": {
        "file": "node_modules/module-with-dynamic-import/index.js",
        "line": 2,
        "column": 4,
        "length": 51,
        "lineText": "const locale = require(`./locale/${language}.json`)"
      }
    }
  ]
}

This would keep these cases out of the terminal as per the current behaviour, but when coupled with the formatMessages API introduced in v0.10.1 it would allow consumers to go through this list and manually throw a warning for each import, effectively restoring the functionality that was lost in v0.11.11.

2. Solving unresolved imports

2.1. Including additional files

With the data above, one could try to parse each expression after esbuild finishes and fix the bundle by including any paths that may be referenced by the dynamic expression. In the example above, which is the exact case shown in Webpack's dynamic expressions documentation section, the bundle could be fixed by including the locale directory with all its files, so that the path can be successfully resolved at runtime.

There is still a lingering problem here, which is that after bundling paths are no longer namespaced to each module. For example, "./locales" originally resolved to node_modules/module-with-dynamic-import/locale, so even if another module also had a locales directory there would be no conflict. After bundling, we'd need to include both directories as siblings to the bundle file, so the name locales would generate a conflict.

Questions:

• Could we extend the plugin API to also receive this type of import, giving consumers the ability to modify the expressions such that they're namespaced to a unique directory?
• Could esbuild leverage its own AST and attempt to provide additional information about these expressions, potentially solving simpler cases like the Webpack example above? Or is this something that consumers would be entirely responsible for, either via a plugin or outside of esbuild entirely?

2.2. Externalising modules with unresolved imports

esbuild could have some sort of safe mode where any paths containing unresolved imports are automatically flagged as external. Using the mongoose example referenced in #1155:

require('./driver').set(require(global.MONGOOSE_DRIVER_PATH));

When in safe mode, esbuild would stop traversing mongoose and would mark it as external, leaving its require as is. Consumers would know to include the entire node_modules/mongoose directory alongside their bundle, provided that external modules are also added to the metafile, as proposed in #972.

2.3. Using incremental for a second pass

Failing the two options above, and provided that unresolved imports are added to the metafile, one could use incremental to run a second pass of the build by looking for unresolved imports resulting from the first pass and flagging them as external on the second pass.

I don't think this is currently possible, since there's no way to change the options passed to the build API and thus making it impossible to inject additional external modules in between runs. But I still wanted to present this option in case there's room to change anything here in the future.

---

I'll be more than happy to contribute with a pull request, but I'd need some guidance on which approach (if any) is more aligned with the project's vision and roadmap, to avoid spending time on a PR that will not get merged.

Thanks in advance!

[evanw]

Thanks for providing such detailed thoughts about this issue. However, I'm already planning to solve this through plugins instead. This is already tracked by issue #700 so I'm going to close this issue as a duplicate of that issue.