|
| 1 | +# Revised Incremental PR Plan for packages/enhanced Changes |
| 2 | + |
| 3 | +## Overview |
| 4 | +Based on a detailed diff analysis, this document provides a more accurate breakdown of changes into focused, incremental PRs. Each PR represents a distinct feature, fix, or refactor that can be merged independently. |
| 5 | + |
| 6 | +## Updated PR Sequence |
| 7 | + |
| 8 | +### PR 1: Runtime Safety Fixes |
| 9 | +**Size**: Tiny (~2 files) |
| 10 | +**Risk**: Low |
| 11 | +**Type**: Fix |
| 12 | +**Feature**: Add defensive checks to prevent runtime errors |
| 13 | + |
| 14 | +**Files to include**: |
| 15 | +- `src/lib/container/runtime/EmbedFederationRuntimeModule.ts` (add `typeof prevStartup === 'function'` check) |
| 16 | +- `src/lib/startup/StartupHelpers.ts` (add `typeof __webpack_require__.x === "function"` check) |
| 17 | + |
| 18 | +**Why first**: These are independent safety fixes that improve stability without any dependencies. |
| 19 | + |
| 20 | +--- |
| 21 | + |
| 22 | +### PR 2: Hook Renaming and Cleanup |
| 23 | +**Size**: Small (~6 files) |
| 24 | +**Risk**: Low (internal refactoring only - all usages updated) |
| 25 | +**Type**: Refactor |
| 26 | +**Feature**: Rename container hooks for clarity and consistency |
| 27 | + |
| 28 | +**Files to include**: |
| 29 | +- `src/lib/container/ContainerPlugin.ts` (rename hooks, add `addRemoteDependency`) |
| 30 | +- `src/lib/container/runtime/FederationRuntimePlugin.ts` |
| 31 | +- `src/lib/container/runtime/FederationModulesPlugin.ts` |
| 32 | +- `src/lib/container/runtime/EmbedFederationRuntimePlugin.ts` |
| 33 | +- `src/lib/container/RemoteModule.ts` (use new hook) |
| 34 | +- Any other files that reference these hooks |
| 35 | + |
| 36 | +**Changes**: |
| 37 | +- `addContainerEntryModule` → `addContainerEntryDependency` |
| 38 | +- `addFederationRuntimeModule` → `addFederationRuntimeDependency` |
| 39 | +- Add new `addRemoteDependency` hook |
| 40 | + |
| 41 | +**Implementation**: |
| 42 | +```javascript |
| 43 | +// Direct rename - all usages updated in same PR |
| 44 | +compiler.hooks.addContainerEntryDependency = new SyncHook([...]); |
| 45 | +compiler.hooks.addFederationRuntimeDependency = new SyncHook([...]); |
| 46 | +compiler.hooks.addRemoteDependency = new SyncHook([...]); |
| 47 | +``` |
| 48 | + |
| 49 | +**Note**: This is NOT a breaking change because all hook usages within the codebase are updated in the same PR. |
| 50 | +--- |
| 51 | + |
| 52 | +### PR 3: Enhanced HoistContainerReferencesPlugin |
| 53 | +**Size**: Medium (~3 files) |
| 54 | +**Risk**: Medium |
| 55 | +**Type**: Feature/Fix |
| 56 | +**Feature**: Improve module hoisting for runtime chunks |
| 57 | + |
| 58 | +**Files to include**: |
| 59 | +- `src/lib/container/HoistContainerReferencesPlugin.ts` (complete rewrite) |
| 60 | +- `test/compiler-unit/container/HoistContainerReferencesPlugin.test.ts` (new tests) |
| 61 | +- Related test fixtures |
| 62 | + |
| 63 | +**Depends on**: PR 2 (needs renamed hooks) |
| 64 | + |
| 65 | +**Key improvements**: |
| 66 | +- Separate handling for container, federation, and remote dependencies |
| 67 | +- Better support for `runtimeChunk: 'single'` configuration |
| 68 | +- Proper remote module hoisting |
| 69 | + |
| 70 | +--- |
| 71 | + |
| 72 | +### PR 4: Basic Share Filtering - Include/Exclude by Version |
| 73 | +**Size**: Medium (~12 files) |
| 74 | +**Risk**: Low |
| 75 | +**Type**: Feature |
| 76 | +**Feature**: Filter shared modules by version constraints (FIXED: Now correctly reads actual module versions from package.json instead of comparing version ranges) |
| 77 | + |
| 78 | +**Files to include**: |
| 79 | +- `src/lib/sharing/utils.ts` (add `testRequestFilters`, `addSingletonFilterWarning`) |
| 80 | +- `src/lib/sharing/ConsumeSharedPlugin.ts` (add version-based filtering) |
| 81 | +- `src/lib/sharing/ProvideSharedPlugin.ts` (add version-based filtering) |
| 82 | +- `src/schemas/sharing/ConsumeSharedPlugin.json` (add include/exclude schema) |
| 83 | +- `src/schemas/sharing/ConsumeSharedPlugin.ts` |
| 84 | +- `src/schemas/sharing/ConsumeSharedPlugin.check.ts` |
| 85 | +- `src/schemas/sharing/ProvideSharedPlugin.json` |
| 86 | +- `src/schemas/sharing/ProvideSharedPlugin.ts` |
| 87 | +- `src/schemas/sharing/ProvideSharedPlugin.check.ts` |
| 88 | +- `test/unit/sharing/utils-filtering.test.ts` (new) |
| 89 | +- `test/configCases/sharing/share-multiple-versions-include/*` (new) |
| 90 | +- `test/configCases/sharing/share-multiple-versions-exclude/*` (new) |
| 91 | + |
| 92 | +**API**: |
| 93 | +```javascript |
| 94 | +shared: { |
| 95 | + react: { |
| 96 | + include: { version: "^18.0.0" }, |
| 97 | + exclude: { version: "17.x" } |
| 98 | + } |
| 99 | +} |
| 100 | +``` |
| 101 | + |
| 102 | +--- |
| 103 | + |
| 104 | +### PR 5: Request Pattern Filtering |
| 105 | +**Size**: Small (~8 files) |
| 106 | +**Risk**: Low |
| 107 | +**Type**: Feature |
| 108 | +**Feature**: Filter shared modules by request patterns |
| 109 | + |
| 110 | +**Files to include**: |
| 111 | +- `src/lib/sharing/ConsumeSharedPlugin.ts` (add request pattern support) |
| 112 | +- `src/lib/sharing/ProvideSharedPlugin.ts` (add request pattern support) |
| 113 | +- Update schemas for request patterns |
| 114 | +- `test/configCases/sharing/prefix-share-filter/*` (new) |
| 115 | +- Related unit tests |
| 116 | + |
| 117 | +**Depends on**: PR 4 (builds on filtering infrastructure) |
| 118 | + |
| 119 | +**API**: |
| 120 | +```javascript |
| 121 | +shared: { |
| 122 | + "@scope/*": { |
| 123 | + include: { request: /^@scope\/[^\/]+$/ } |
| 124 | + } |
| 125 | +} |
| 126 | +``` |
| 127 | + |
| 128 | +--- |
| 129 | + |
| 130 | +### PR 6: Fallback Version Support |
| 131 | +**Size**: Small (~6 files) |
| 132 | +**Risk**: Low |
| 133 | +**Type**: Feature |
| 134 | +**Feature**: Add fallback version checking for filters |
| 135 | + |
| 136 | +**Files to include**: |
| 137 | +- `src/lib/sharing/ConsumeSharedPlugin.ts` (add fallbackVersion logic) |
| 138 | +- `src/lib/sharing/ProvideSharedPlugin.ts` (add fallbackVersion logic) |
| 139 | +- Schema updates for fallbackVersion |
| 140 | +- Unit tests for fallback version |
| 141 | +- Integration tests |
| 142 | + |
| 143 | +**Depends on**: PR 4 |
| 144 | + |
| 145 | +**API**: |
| 146 | +```javascript |
| 147 | +shared: { |
| 148 | + react: { |
| 149 | + include: { |
| 150 | + version: "^18.0.0", |
| 151 | + fallbackVersion: "^17.0.0" |
| 152 | + } |
| 153 | + } |
| 154 | +} |
| 155 | +``` |
| 156 | + |
| 157 | +--- |
| 158 | + |
| 159 | +### PR 7: nodeModulesReconstructedLookup Feature |
| 160 | +**Size**: Medium (~10 files) |
| 161 | +**Risk**: Low |
| 162 | +**Type**: Feature |
| 163 | +**Feature**: Enable path reconstruction for node_modules resolution |
| 164 | + |
| 165 | +**Files to include**: |
| 166 | +- `src/lib/sharing/utils.ts` (add `extractPathAfterNodeModules`) |
| 167 | +- `src/lib/sharing/ConsumeSharedPlugin.ts` (add two-stage lookup) |
| 168 | +- `src/lib/sharing/ProvideSharedPlugin.ts` (add two-stage lookup) |
| 169 | +- Schema updates for nodeModulesReconstructedLookup |
| 170 | +- `test/configCases/sharing/share-deep-module/*` (new) |
| 171 | +- Related unit tests |
| 172 | + |
| 173 | +**Depends on**: PR 4 (uses filtering infrastructure) |
| 174 | + |
| 175 | +--- |
| 176 | + |
| 177 | +### PR 8: SharePlugin - Unified API |
| 178 | +**Size**: Medium (~8 files) |
| 179 | +**Risk**: Low |
| 180 | +**Type**: Feature |
| 181 | +**Feature**: New SharePlugin that combines consume and provide |
| 182 | + |
| 183 | +**Files to include**: |
| 184 | +- `src/lib/sharing/SharePlugin.ts` (add schema validation) |
| 185 | +- `src/index.ts` (export SharePlugin) |
| 186 | +- `src/schemas/sharing/SharePlugin.json` (new) |
| 187 | +- `src/schemas/sharing/SharePlugin.ts` (new) |
| 188 | +- `src/schemas/sharing/SharePlugin.check.ts` (new) |
| 189 | +- `test/unit/sharing/SharePlugin.test.ts` |
| 190 | +- `test/compiler-unit/sharing/SharePlugin.test.ts` (new) |
| 191 | + |
| 192 | +**Depends on**: PR 4-7 (passes through all filtering options) |
| 193 | + |
| 194 | +--- |
| 195 | + |
| 196 | +### PR 9: Enhanced Layer Support |
| 197 | +**Size**: Small (~6 files) |
| 198 | +**Risk**: Low |
| 199 | +**Type**: Feature/Fix |
| 200 | +**Feature**: Improve layer handling and issuerLayer fallback |
| 201 | + |
| 202 | +**Files to include**: |
| 203 | +- `src/lib/sharing/ConsumeSharedModule.ts` (layer parameter) |
| 204 | +- `src/lib/sharing/ConsumeSharedFallbackDependency.ts` (layer support) |
| 205 | +- `src/lib/sharing/resolveMatchedConfigs.ts` (issuerLayer priority) |
| 206 | +- `src/lib/sharing/utils.ts` (createLookupKeyForSharing) |
| 207 | +- Layer-specific tests |
| 208 | +- Unit tests for issuerLayer fallback (PR #3893) |
| 209 | + |
| 210 | +--- |
| 211 | + |
| 212 | +### PR 10: Module Exports and API Surface |
| 213 | +**Size**: Tiny (~3 files) |
| 214 | +**Risk**: Low |
| 215 | +**Type**: Feature |
| 216 | +**Feature**: Export internal modules for advanced usage |
| 217 | + |
| 218 | +**Files to include**: |
| 219 | +- `src/index.ts` (add ConsumeSharedModule, ProvideSharedModule exports) |
| 220 | +- Declaration file updates |
| 221 | +- Documentation |
| 222 | + |
| 223 | +--- |
| 224 | + |
| 225 | +### PR 11: Comprehensive Test Suite |
| 226 | +**Size**: Large (test-only) |
| 227 | +**Risk**: None |
| 228 | +**Type**: Test |
| 229 | +**Feature**: Additional test coverage and edge cases |
| 230 | + |
| 231 | +**Files to include**: |
| 232 | +- Remaining test files not included in feature PRs |
| 233 | +- `test/helpers/webpack.ts` |
| 234 | +- Additional unit test coverage |
| 235 | +- Edge case tests |
| 236 | + |
| 237 | +**Depends on**: All feature PRs |
| 238 | + |
| 239 | +--- |
| 240 | + |
| 241 | +### PR 12: Package Updates and Cleanup |
| 242 | +**Size**: Small |
| 243 | +**Risk**: Low |
| 244 | +**Type**: Chore |
| 245 | +**Feature**: Update dependencies and final cleanup |
| 246 | + |
| 247 | +**Files to include**: |
| 248 | +- `package.json` |
| 249 | +- `pnpm-lock.yaml` |
| 250 | +- `.cursorrules` (editor configuration file) |
| 251 | +- `src/scripts/compile-schema.js` (if needed) |
| 252 | + |
| 253 | +## Key Insights from Analysis |
| 254 | + |
| 255 | +1. **Runtime Safety Fixes** are completely independent and should go first |
| 256 | +2. **Hook Renaming** is a prerequisite for the hoisting improvements |
| 257 | +3. **Share Filtering** can be broken into smaller pieces: |
| 258 | + - Version filtering (core functionality) |
| 259 | + - Request pattern filtering (builds on version) |
| 260 | + - Fallback version support (enhancement) |
| 261 | + - nodeModulesReconstructedLookup (separate feature) |
| 262 | +4. **Layer Support** improvements are somewhat independent but share some utilities |
| 263 | +5. **Test files** are well-organized and can be included with their respective features |
| 264 | + |
| 265 | +## Dependency Graph |
| 266 | + |
| 267 | +``` |
| 268 | +PR 1 (Runtime Fixes) ──────────────────> (Independent) |
| 269 | +
|
| 270 | +PR 2 (Hook Renaming) ──────────────────> PR 3 (Hoisting) |
| 271 | +
|
| 272 | +PR 4 (Version Filter) ──┬──> PR 5 (Request Filter) |
| 273 | + ├──> PR 6 (Fallback Version) |
| 274 | + └──> PR 7 (nodeModules Lookup) ──> PR 8 (SharePlugin) |
| 275 | +
|
| 276 | +PR 9 (Layer Support) ──────────────────> (Semi-independent) |
| 277 | +
|
| 278 | +PR 10 (Exports) ───────────────────────> (Independent) |
| 279 | +
|
| 280 | +All Feature PRs ───────────────────────> PR 11 (Tests) ──> PR 12 (Cleanup) |
| 281 | +``` |
| 282 | + |
| 283 | +## Benefits of This Revised Plan |
| 284 | + |
| 285 | +1. **Clearer Separation**: Each PR has a distinct purpose |
| 286 | +2. **Reduced Risk**: Smaller, focused changes are easier to review and test |
| 287 | +3. **Flexibility**: Some PRs can be developed in parallel |
| 288 | +4. **Progressive Enhancement**: Each filtering feature builds on the previous |
| 289 | +<<<<<<< HEAD |
| 290 | +5. **Early Wins**: Runtime fixes and hook renaming can be merged quickly |
| 291 | +======= |
| 292 | +5. **Early Wins**: Runtime fixes and hook renaming can be merged quickly |
| 293 | +>>>>>>> origin/refactor/hook-renaming-cleanup-v2 |
0 commit comments