{ external: false } in resolveId passed incorrectly

[guybedford]

When returning { id, external: true } from the resolveId, everything works perfectly as expected.

I was trying to work to a single function shape though somewhat in the name of performance, but when passing { id, external: false } this seems to propogate as-is to the load hook and the rest of the pipeline.

It should be a simple one-liner to fix along the lines of resolved = resolved.external === false ? resolved.id : resolved sort of thing.

Sorry I can't PR right now too :)

[lukastaegert]

There are some issues around this so I am not sure which one you refer to. I list the ones that I stumbled upon, please note if the one you refer to is among them (noting down an "expected behaviour"/"actual behaviour" as suggested by the issue template is really helpful!).

• the resolveId context function does not return just string | void but anything that is returned by the resolveId plugin hook, which includes false for external dependencies or an object. I guess there should be a mapping for objects but I am not sure about false. I would actually return the unmodified id in that case and leave it to the plugin to run it through the isExternal context function.
• the resolveDynamicImport plugin hook is treated internally as if it only returns string | void but at the moment, it just again uses the resolveId context function by default with all the issues listed above. My suggestion here would be to allow resolveDynamicImport to have the full spectrum of valid resolveId plugin hook return values and adapt the internal logic accordingly. The default plugin could use the resolveId and isExternal context functions together to construct an object.

[guybedford]

Yes it seems it is both issues - I hit the resolveDynamicImport bug here as well as soon as there was a dynamic import that was returning { external: true } through the normal resolveId (and this was even without any resolveDynamicImport hook in the plugin to begin with.

[lukastaegert]

I finally posted a fix at #2829. Would be nice if you could have a look as I also added a new plugin context function with the prospect of deprecating this.resolveId and this.isExternal at some point.