chore(rsbuild-plugin): split setUp function to help extend #3215
Conversation
🦋 Changeset detectedLatest commit: cd5269c 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. |
2heal1
force-pushed
the
chore/split-rsbuild-plugin
branch
2 times, most recently
from
de22c3e to
c8f070d
Compare
2heal1
force-pushed
the
chore/split-rsbuild-plugin
branch
from
c1a5841 to
983b487
Compare
2heal1
force-pushed
the
chore/split-rsbuild-plugin
branch
3 times, most recently
from
e31faef to
6623262
Compare
|
It will be merged in two weekes |
2heal1
force-pushed
the
chore/split-rsbuild-plugin
branch
from
6623262 to
2f4fef8
Compare
![squadronai[bot]](https://avatars.githubusercontent.com/in/901096?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This pull request focuses on improving the extensibility and maintainability of the ModuleFederationPlugin by introducing a centralized reference to the plugin's name. The changes span across multiple files in the packages/enhanced and packages/rspack directories, and they involve the following key updates:
- A new
PLUGIN_NAMEconstant has been introduced and exported from various modules, includingModuleFederationPlugin. This allows for a consistent and easily accessible reference to the plugin's name, which can be useful for logging, configuration, or integration with other parts of the application. - The
nameproperty of theModuleFederationPluginclass has been updated to use thePLUGIN_NAMEconstant, further centralizing the plugin's name and making it more easily reusable across the codebase. - The new
PLUGIN_NAMEexports enable better extensibility and customization of the plugin, as users can now access the plugin's name directly without having to rely on implicit references or internal implementation details.
These changes aim to improve the overall maintainability and scalability of the ModuleFederationPlugin by providing a more structured and extensible approach to managing the plugin's name and related functionality.
File Summaries
| File | Summary |
|---|---|
| packages/enhanced/src/index.ts | The code changes introduce a new export, PLUGIN_NAME, from the ModuleFederationPlugin module. This change likely aims to provide a centralized and easily accessible reference to the plugin's name, which can be useful for various purposes, such as logging, configuration, or integration with other parts of the application. |
| packages/enhanced/src/rspack.ts | The code changes introduce a new export for the PLUGIN_NAME constant from the @module-federation/rspack/plugin module. This change likely aims to provide additional functionality or information related to the ModuleFederationPlugin that is being exported. |
| packages/enhanced/src/webpack.ts | The code changes introduce a new constant PLUGIN_NAME and export it alongside the ModuleFederationPlugin from the ./wrapper/ModuleFederationPlugin module. This modification likely aims to provide a more explicit and accessible reference to the plugin name, potentially improving the extensibility and maintainability of the codebase. |
| packages/enhanced/src/wrapper/ModuleFederationPlugin.ts | The code changes introduce a new export for the PLUGIN_NAME constant, which was previously defined as a private variable. This change allows the plugin name to be accessed and used outside of the ModuleFederationPlugin class, potentially enabling better extensibility and customization of the plugin. |
| packages/rspack/src/ModuleFederationPlugin.ts | The code changes introduce a new constant PLUGIN_NAME and update the name property of the ModuleFederationPlugin class to use this constant. This change likely aims to centralize the plugin name and make it more easily accessible or reusable across the codebase. |
| packages/rspack/src/index.ts | The code changes introduce a new export, PLUGIN_NAME, from the ModuleFederationPlugin module. This change likely aims to provide a more comprehensive API for the plugin, allowing users to access the plugin's name directly. |
2heal1
force-pushed
the
chore/split-rsbuild-plugin
branch
from
cd3287b to
d0074b0
Compare
There was a problem hiding this comment.
Incremental Review
Comments posted: 8
Configuration
Squadron Mode: essential
Commits Reviewed
2f4fef827f713e9fe279c36153762ea60cb48525...f9f5c25b441783fe6890ed584542ddde1d2bdc37
Files Reviewed
- packages/enhanced/src/index.ts
- packages/enhanced/src/rspack.ts
- packages/enhanced/src/webpack.ts
- packages/enhanced/src/wrapper/ModuleFederationPlugin.ts
- packages/rspack/src/ModuleFederationPlugin.ts
- packages/rspack/src/index.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/angry-spoons-wink.md
- .changeset/spicy-roses-hear.md
- apps/rslib-module/.storybook/main.ts
- apps/rslib-module/package.json
- apps/rslib-module/rslib.config.ts
- packages/modernjs/package.json
- packages/modernjs/src/cli/utils.ts
- packages/rsbuild-plugin/package.json
- packages/rsbuild-plugin/rollup.config.js
- packages/rsbuild-plugin/src/cli/index.ts
- packages/rsbuild-plugin/src/utils/autoDeleteSplitChunkCacheGroups.ts
- packages/rsbuild-plugin/src/utils/index.ts
- packages/storybook-addon/package.json
- packages/storybook-addon/preset.ts
- packages/storybook-addon/src/utils/with-module-federation-enhanced-rsbuild.ts
- pnpm-lock.yaml
| export { | ||
| default as ModuleFederationPlugin, | ||
| PLUGIN_NAME, | ||
| } from './wrapper/ModuleFederationPlugin'; | ||
| export { default as ContainerReferencePlugin } from './wrapper/ContainerReferencePlugin'; | ||
| export { default as SharePlugin } from './wrapper/SharePlugin'; | ||
| export { default as ContainerPlugin } from './wrapper/ContainerPlugin'; |
There was a problem hiding this comment.
The export statement for ContainerPlugin appears to be truncated. Complete the line to properly export the plugin:
| export { default as ContainerPlugin } from './wrapper/ContainerPlugin'; | |
| export { default as ContainerPlugin } from './wrapper/ContainerPlugin'; |
| @@ -1,5 +1,8 @@ | |||
| import type { moduleFederationPlugin } from '@module-federation/sdk'; | |||
There was a problem hiding this comment.
The import statement only imports the type but not the actual module. If the module is needed at runtime, consider importing both:
| import type { moduleFederationPlugin } from '@module-federation/sdk'; | |
| import { moduleFederationPlugin } from '@module-federation/sdk'; |
| import type { moduleFederationPlugin } from '@module-federation/sdk'; | ||
| export { default as ModuleFederationPlugin } from './wrapper/ModuleFederationPlugin'; | ||
| export { | ||
| default as ModuleFederationPlugin, | ||
| PLUGIN_NAME, | ||
| } from './wrapper/ModuleFederationPlugin'; | ||
| export { default as ContainerReferencePlugin } from './wrapper/ContainerReferencePlugin'; | ||
| export { default as SharePlugin } from './wrapper/SharePlugin'; | ||
| export { default as ContainerPlugin } from './wrapper/ContainerPlugin'; |
There was a problem hiding this comment.
Consider adding JSDoc comments for the exported plugins to improve API documentation. This will help users understand the purpose and usage of each plugin. For example:
/**
- Exports core Module Federation plugins for webpack configuration
- @module ModuleFederationPlugins
*/
Also consider grouping related exports together with explanatory comments for better code organization.
| import { | ||
| default as ModuleFederationPlugin, | ||
| PLUGIN_NAME, | ||
| } from './wrapper/ModuleFederationPlugin'; |
There was a problem hiding this comment.
The import statement could be more specific about what's being imported. Instead of using 'default as', consider importing the plugin directly for better clarity and readability. Here's a suggested change:
| import { | |
| default as ModuleFederationPlugin, | |
| PLUGIN_NAME, | |
| } from './wrapper/ModuleFederationPlugin'; | |
| import ModuleFederationPlugin, { PLUGIN_NAME } from './wrapper/ModuleFederationPlugin'; |
This makes it immediately clear what's being imported without the 'default as' indirection.
| export class ModuleFederationPlugin implements RspackPluginInstance { | ||
| readonly name = 'RspackModuleFederationPlugin'; | ||
| readonly name = PLUGIN_NAME; | ||
| private _options: moduleFederationPlugin.ModuleFederationPluginOptions; |
There was a problem hiding this comment.
The type name moduleFederationPlugin.ModuleFederationPluginOptions uses camelCase for the namespace which is unconventional. TypeScript namespaces typically use PascalCase. Consider renaming to ModuleFederationPlugin.ModuleFederationPluginOptions or creating a dedicated types file.
| export class ModuleFederationPlugin implements RspackPluginInstance { | ||
| readonly name = 'RspackModuleFederationPlugin'; | ||
| readonly name = PLUGIN_NAME; | ||
| private _options: moduleFederationPlugin.ModuleFederationPluginOptions; | ||
| private _statsPlugin?: StatsPlugin; |
There was a problem hiding this comment.
The optional _statsPlugin property is using a nullable type with ?. Consider using a more explicit initialization to null or defining it as non-nullable if it's required after initialization. This helps with type safety and makes the intent clearer. ```suggestion
private _statsPlugin: StatsPlugin | null = null;
| @@ -1,4 +1,4 @@ | |||
| export { ModuleFederationPlugin } from './ModuleFederationPlugin'; | |||
| export { ModuleFederationPlugin, PLUGIN_NAME } from './ModuleFederationPlugin'; | |||
| import { container } from '@rspack/core'; | |||
| export const ContainerPlugin = container.ContainerPlugin; | |||
| export const ContainerReferencePlugin = container.ContainerReferencePlugin; | |||
There was a problem hiding this comment.
The ContainerReferencePlugin export appears to be incomplete. The line ends with "Contain" which seems to be truncated. This should be completed to properly export the plugin: ```suggestion
export const ContainerReferencePlugin = container.ContainerReferencePlugin;
| export { ModuleFederationPlugin, PLUGIN_NAME } from './ModuleFederationPlugin'; | ||
| import { container } from '@rspack/core'; | ||
| export const ContainerPlugin = container.ContainerPlugin; | ||
| export const ContainerReferencePlugin = container.ContainerReferencePlugin; |
There was a problem hiding this comment.
Consider grouping related exports and imports together for better code organization. Move the import statement before the exports:
| export { ModuleFederationPlugin, PLUGIN_NAME } from './ModuleFederationPlugin'; | |
| import { container } from '@rspack/core'; | |
| export const ContainerPlugin = container.ContainerPlugin; | |
| export const ContainerReferencePlugin = container.ContainerReferencePlugin; | |
| import { container } from '@rspack/core'; | |
| export { ModuleFederationPlugin, PLUGIN_NAME } from './ModuleFederationPlugin'; | |
| export const ContainerPlugin = container.ContainerPlugin; | |
| export const ContainerReferencePlugin = container.ContainerReferencePlugin; |
2heal1
changed the title
Description
split setUp function to help extend
Related Issue
Types of changes
Checklist