-
Notifications
You must be signed in to change notification settings - Fork 1
[16] Introduce "load strategies" to the webpack package, and update react-pointcuts to cater for dynamic import using "React.lazy". #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
* rename to proper module namespace * update docs links * update versions * web toggle point in readme title * fixup changelog from revised 0.x range * 2.0.0 -> 0.5.0 in oss version scheme * fix broken link syntax in CHANGELOG * consistent quoting * more version history issues * fixup module name in jsdoc * add web remove sdkInstanceProvider * remove SDKInstanceProvider * fixup jsdoc dedupe * tweak * clarity re: ssr package * casing etc
* update workflows * version * typo * update chromium linux snaps * versions for serve update * package.json repository field * update root package.lock * bugs & directories/doc fields * fix changelog --------- Co-authored-by: Tom Pereira <[email protected]>
Co-authored-by: Tom Pereira <[email protected]>
correct errant lint change for useContentEditable remove needless Suspense wrapping content-management example
…tory add note re: use of node scoping in NextJs
|
Phoar, this is a big one 🚨 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a significant architectural change to support different module loading strategies in the webpack package, enabling code-splitting and lazy-loading of variant code. The main updates include:
- Introduction of a "load strategy" concept with three factory implementations (static, deferred require, deferred dynamic import)
- Breaking API changes in the webpack package (plugin rename, configuration property renames, toggle handler → toggle handler factory pattern)
- React.lazy integration in react-pointcuts for code-splitting React components
Key Changes
- Load Strategies: New abstraction allowing variants to be loaded statically (at bootstrap), synchronously on-demand (deferred require), or asynchronously with code-splitting (dynamic import)
- API Refactoring: Toggle handlers converted to factories receiving
pack/unpackfunctions, enabling deferred execution and lazy loading patterns - React Integration: Added lazy component detection and Suspense wrapping with useDeferredValue support for smooth transitions between variants
Reviewed changes
Copilot reviewed 115 out of 119 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/webpack/src/plugins/index.js | Renames plugin export to TogglePointInjectionPlugin |
| packages/webpack/src/plugins/togglePointInjection/fillDefaultOptionalValues.js | Adds default load strategy and updates configuration defaults |
| packages/webpack/src/plugins/togglePointInjection/schema.json | Updates schema for new configuration structure |
| packages/webpack/src/moduleLoadStrategyFactories/*.js | Implements three load strategy factories |
| packages/webpack/src/toggleHandlerFactories/pathSegment.js | Converts handler to factory pattern with pack/unpack support |
| packages/react-pointcuts/src/withTogglePointFactory/index.js | Adds lazy component detection and Suspense wrapping |
| packages/react-pointcuts/src/lazyComponentLoadStrategyFactory.js | New strategy factory wrapping modules in React.lazy |
| test/automation/package.json | Adds UI mode scripts and fixes express test path |
| examples//src/**/.js | Updates all examples to use new API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Issue
Allow for different module loading strategies, adding the ability to code-split variation code.
resolves #16
Details
webpackpackage updatesUpdate the webpack plugin to ingest a strategy for generating import code, such that each point cut may define how code is accessed at run-time.
At present, all code is statically required at application bootstrap, via the use of
import.meta.webpackContext. Unadorned, this method causes all variation modules in a potential join points to be statically imported, thus all side-effects of all modules are run. This requires toggled code to be side-effect free, or have side-effects that do not interfere with each other / can act idempotently, when identical. It also means all the varied code is included in the bundle that contains the base/varied files, potentially bloating the base experience.Although the
import.meta.webpackContextmethod supports adornment with a "mode" parameter that can takelazyandeageras a means of deferring import, these necessarily output aPromise, thus can only be compatible with consuming code that is asynchronously importing the module to be varied.To support other schemes, (such as a deferred
requiresupported by bundled CommonJS, or perhaps an ESM equivalent in future - see issue), the plugin has been updated to accept an optionalloadStrategyconfiguration parameter. These strategies manually provide that which the webpack context module api afforded, preparing all potential codepaths based on the variations found by the point cut configuration.load strategy factories
Three factories for such strategies are provided, as package exports:
staticLoadStrategyFactoryThis produces code equivalent to the
import.meta.webpackContext/ context module api scheme (so parity with the current version), whereby all variant modules are statically imported at the point the chunk entrypoint is imported. At this time, all side-effects will execute in series.This is no longer the default option, since the common case assumes the overhead and impact of side-effects for an irrelevant module selection should be avoided. However, if parity with the existing scheme is deemed desirable, perhaps to avoid costly just-in-time code execution, this option is still available.
deferredRequireLoadStrategyFactoryThis produces modules wrapped in a synchronous function, that will
requirethe module at the point that it's selected.Despite using
require(a commonJs feature), it appears compatible with output.module, with theserveexample updated to use this output format.This is the default load strategy, if one is not specified.
N.B. Due to oddness in NextJs 14 see here this produces
Promiseout of the Webpack build, so appears incompatible. Next 15 (app or pages router) does not appear to suffer the same.deferredDynamicImportLoadStrategyFactoryThis produces modules wrapped in an asynchronous function that dynamically imports the module at the point that it's selected. This hence returns a
Promise, and the toggle point needs to be compatible with asynchronous access.Webpack, unless overridden, will code-split the point cuts, creating non-initial chunks. This should ensure that the "base" version of the application has no increased initial chunk size.
Strategy factory modules
Since a strategy needs to represent both code that is executed at compile time, and provide code that is baked into the compilation to be executed at run-time, the interface of these modules is somewhat strange.
The default export represents the factory for a
importCodeGenerator, used to generate the import code itself at compile-time.It can also have named exports that are baked into the compilation to "pack" and "unpack" the modules that the
importCodeGeneratorhas accessed. This provides the mean to defer import and/or execution, or otherwise mutate the module at the point of storage in "feature store", and mutate back into an executable form, when it's deemed the module is relevant for the toggle state.react-pointcutspackage updatesThe package provides it's own load strategy factory, that composes the
deferredDynamicImportLoadStrategyFactory, for itsimportCodeGeneratormethod, but setting the "pack" option to wrap the dynamic import statement (comprising a load function) inReact.lazy.The
withToggleHandlerFactoryis updated to detect lazy-wrapped modules, wrapping in Suspense if detected. The suspense boundary is backed by anullfallback, to allow server-side rendering to maintain the existing markup as a lazy bundle is downloaded prior to hydration.Where available (React 18+) it also wraps the component in
useDeferredValuesuch that reactive feature stores can change the selected variation whilst maintaining the mount of the prior variation as new chunks are downloaded. This avoids any "flash of no content" as the variant selection is changing.Updated the
withToggledHookFactoryto support "unpacking" of modules.Scout Rule
webackpackageTogglePointInjectionplugin to have aPluginsuffix, supporting a standard webpack naming conventiontoggleHandlerpackage exports to be "factories", now available under atoggleHandlerFactories/exports pathnextpeer dependency, there's no reason to be explicitnode_modulespart of the app root, and something odd has happened) are skippedreact-pointcutspackagetest/automationuiscripts for playwright ui modefeaturespackagessrBackedReactContextstoreserveexampleproductionwebpack mode withsource-mapdevtool andsource-map-loader, for clarity when using dev toolsnextexampleREADME.mdxfilestoggle-point.d.tsfromtsconfig.jsonexpressexamplesource-mapdevtool andsource-map-loader, for clarity when using dev toolsoutput.module, to help demonstrate this compatibilityrepo root
reactandreact-domfrom repo root package.json, no longer appears required20.8sinceimport.meta.resolveused in some packages / exampleseslint-plugin-workspacesto0.11.0after addition of ESLint9 supportUpgrade guide
This change will represent breaking changes in consumers of the webpack package.
TogglePointInjectionnamed export is now namedTogglePointInjectionPluginThe point cut configuration needs to be modified for the plugin thus:
togglePointModuleoption is renamed totogglePointModuleSpecifiertoggleHandleroption is renamed totoggleHandlerFactoryModuleSpecifierN.B. The existing code has a "load strategy" based on
import.meta.webpackContext.For complete parity with this, a strategy built by the
staticLoadStrategyFactoryshould be specified (as theloadStrategy), but would suggest trying the new default which should be functionally identical, and avoid needless side-effects. However, the pages router in NextJs 14 and below appears to convertrequire()intoimport()(see issue, and above), so the default should be avoided in this setup.CheckList
main.