fix(manifest): stats should add prefetchInterface if enable dataPrefetch#3327
fix(manifest): stats should add prefetchInterface if enable dataPrefetch#3327
Conversation
🦋 Changeset detectedLatest commit: 88dc835 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Summary
This pull request addresses an issue with the stats and manifest functionality, ensuring that the prefetchInterface property is correctly set when the dataPrefetch feature is enabled. The changes made in this pull request are as follows:
- The
ManifestManagerclass has been updated to set theprefetchInterfaceproperty in thestats.metaDataobject based on thedataPrefetchflag, rather than checking for the existence of aprefetchFilePath. - The
StatsManagerclass has been updated to include a check to determine if theprefetchInterfaceshould be included in themetaDataobject, based on the existence of a file at a specific path. This ensures that theprefetchInterfaceis correctly included in themetaDataobject, which is likely used for further processing or reporting.
These changes help to improve the overall functionality and reliability of the stats and manifest features, ensuring that the prefetchInterface is properly handled when the dataPrefetch feature is enabled.
File Summaries
| File | Summary |
|---|---|
| packages/manifest/src/ManifestManager.ts | The code changes update the ManifestManager class to ensure that the prefetchInterface property is set correctly in the stats.metaData object. The changes remove the previous logic that checked for the existence of a prefetchFilePath and instead rely on the dataPrefetch flag to determine whether to set the prefetchInterface property. |
| packages/manifest/src/StatsManager.ts | The code changes add a check to determine if the prefetchInterface should be included in the metaData object. It checks for the existence of a file at a specific path and sets the prefetchInterface flag accordingly. This ensures that the prefetchInterface is correctly included in the metaData object, which is likely used for further processing or reporting. |
There was a problem hiding this comment.
Incremental Review
Comments posted: 3
Configuration
Squadron Mode: essential
Commits Reviewed
cafc5f040274ccb274f13d16910146415e8bd973...88dc8353b5c90a3fcee15f16c922982d14b85a0d
Files Reviewed
- packages/manifest/src/StatsManager.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/shaggy-ligers-teach.md
| let prefetchInterface = false; | ||
| const prefetchFilePath = path.resolve( | ||
| compiler.options.context || process.cwd(), | ||
| `node_modules/.mf/${encodeName(name!)}/${MFPrefetchCommon.fileName}`, | ||
| ); | ||
| const existPrefetch = fs.existsSync(prefetchFilePath); | ||
| if (existPrefetch) { | ||
| const content = fs.readFileSync(prefetchFilePath).toString(); | ||
| if (content) { | ||
| prefetchInterface = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
The prefetch interface check could be extracted into a separate method for better readability and reusability. Here's a suggested refactor:
| let prefetchInterface = false; | |
| const prefetchFilePath = path.resolve( | |
| compiler.options.context || process.cwd(), | |
| `node_modules/.mf/${encodeName(name!)}/${MFPrefetchCommon.fileName}`, | |
| ); | |
| const existPrefetch = fs.existsSync(prefetchFilePath); | |
| if (existPrefetch) { | |
| const content = fs.readFileSync(prefetchFilePath).toString(); | |
| if (content) { | |
| prefetchInterface = true; | |
| } | |
| } | |
| private checkPrefetchInterface(compiler: Compiler, name: string): boolean { | |
| const prefetchFilePath = path.resolve( | |
| compiler.options.context || process.cwd(), | |
| `node_modules/.mf/${encodeName(name)}/${MFPrefetchCommon.fileName}`, | |
| ); | |
| if (!fs.existsSync(prefetchFilePath)) { | |
| return false; | |
| } | |
| const content = fs.readFileSync(prefetchFilePath).toString(); | |
| return !!content; | |
| } |
Then use it as: metaData.prefetchInterface = this.checkPrefetchInterface(compiler, name!);
| const prefetchFilePath = path.resolve( | ||
| compiler.options.context || process.cwd(), | ||
| `node_modules/.mf/${encodeName(name!)}/${MFPrefetchCommon.fileName}`, | ||
| ); |
There was a problem hiding this comment.
The path resolution could be improved by using a more robust approach to handle path separators across different operating systems. Consider using path.join() instead of path.resolve():
| const prefetchFilePath = path.resolve( | |
| compiler.options.context || process.cwd(), | |
| `node_modules/.mf/${encodeName(name!)}/${MFPrefetchCommon.fileName}`, | |
| ); | |
| const prefetchFilePath = path.join( | |
| compiler.options.context || process.cwd(), | |
| 'node_modules', | |
| '.mf', | |
| encodeName(name!), | |
| MFPrefetchCommon.fileName | |
| ); |
| const content = fs.readFileSync(prefetchFilePath).toString(); | ||
| if (content) { | ||
| prefetchInterface = true; | ||
| } |
There was a problem hiding this comment.
The file reading operation should include error handling to prevent crashes from file system issues:
| const content = fs.readFileSync(prefetchFilePath).toString(); | |
| if (content) { | |
| prefetchInterface = true; | |
| } | |
| try { | |
| const content = fs.readFileSync(prefetchFilePath).toString(); | |
| if (content) { | |
| prefetchInterface = true; | |
| } | |
| } catch (error) { | |
| console.warn(`Failed to read prefetch interface file: ${error.message}`); | |
| prefetchInterface = false; | |
| } |
These changes will make the code more robust and maintainable.
Description
the stats and manifest should both add
prefetchInterfaceif enabledataPrefetchRelated Issue
Types of changes
Checklist