fix(enhanced): Add fallback parsing for container module exposes (#4034) #4083
Conversation
Co-authored-by: Zack Jackson <[email protected]>
|
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@codex see if there is a better way to fix this issue / the root cause of the problem presented here that cuases the need for this kind of fix in the first place. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| try { | ||
| // Prefer parsing exposes from stats first | ||
| entries = JSON.parse(data[3]); | ||
| } catch { | ||
| // If that fails, fallback to the original options | ||
| const exposes = this._options.exposes; | ||
|
|
||
| if (!exposes || typeof exposes !== 'object') { | ||
| return; | ||
| } | ||
|
|
||
| entries = Object.entries(exposes); | ||
| } | ||
|
|
||
| entries.forEach(([prefixedName, file]) => { | ||
| // TODO: support multiple import | ||
| exposesMap[getFileNameWithOutExt(file.import[0])] = getExposeItem({ |
There was a problem hiding this comment.
[P1] Normalize fallback exposes before accessing import
When JSON parsing of identifier fails, the code falls back to this._options.exposes and iterates the entries assuming each value already looks like { import: [...] }. For typical module federation configs the values are plain strings (e.g. exposes: { './foo': './foo.js' }), so file is a string and file.import[0] throws, causing the Stats plugin to crash in the same spaces-in-path scenario this change is supposed to fix. The fallback needs to normalize string/array expose definitions into an object with an import array (or skip them) before dereferencing file.import.
Useful? React with 👍 / 👎.
|
Summary
Testing
|
|
@vinicius-at-webflow hows this? |
|
@2heal1 I see you went for an unified approach instead of the fallback one, keeping the |
|
Still a workaround but should buy time till we can rewrite this. Pulling data from the stats at all is very slow. Considering we more recently added "communication hooks" to federation, we actually know what modules are registered at a high level. So we should loop over a set of known modules to extract data instead of serializing the whole graph in stats to pull the information. |
Description
The
_handleContainerModulemethod frompackages/manifest/src/ModuleHandler.tscallssplit(' ')on the module'sidentifierwhich breaks if the path or expose key contain spaces or other special characters.Here's a an example of a problematic identifier (see the
importpath for theButtonexpose):Related Issue
#2684
Types of changes
Checklist