fix(runtime): retry lazy component load after a failed dynamic import - #6772
Open
yasinkocak wants to merge 4 commits into
Open
fix(runtime): retry lazy component load after a failed dynamic import#6772yasinkocak wants to merge 4 commits into
yasinkocak wants to merge 4 commits into
Conversation
A single failed dynamic import() of a lazy component's *.entry.js chunk (dropped network request, flaky mobile connection, etc.) permanently bricks that host element for the rest of the page's lifetime: no rendered content, no lifecycle callbacks, and no recovery -- not even for a brand-new element of the same tag -- until a full page reload. Root cause: HOST_FLAGS.hasInitializedComponent is set before the load attempt and never cleared on failure, and HOST_FLAGS.hasConnected (set unconditionally on first connect) blocks connected-callback.ts from ever retrying. On top of that, the browser's own per-document module map caches the failed import() specifier, so even a bare retry of the same bundle would never reach the network again. This clears hasInitializedComponent when the lazy constructor isn't found, lets connected-callback.ts retry initialization on the next reconnect, and cache-busts the retried import() URL (the same pattern already used for ?s-hmr=) so the retry actually reaches the network. Fixes stenciljs#6771
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Stencil’s runtime resilience for lazy-loaded components by allowing recovery after a transient dynamic import() failure (e.g., flaky network / blocked request), instead of leaving the host element permanently unable to initialize for the lifetime of the page.
Changes:
- Clear
HOST_FLAGS.hasInitializedComponentwhen a lazy-load constructor cannot be resolved, enabling future retry attempts. - Retry
initializeComponent()on disconnect+reconnect when a prior initialization attempt failed. - Add an
?s-retry=Ncache-busting query param on repeated lazy-bundle import attempts to avoid browser module-map caching of failed imports. - Add regression tests covering flag reset and reconnect-triggered retry behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/runtime/test/lazy-load-retry.spec.tsx | Adds regression tests for failed lazy-load recovery via flag clearing + reconnect retry. |
| src/runtime/initialize-component.ts | Clears hasInitializedComponent when constructor resolution fails so the host can retry later. |
| src/runtime/connected-callback.ts | Re-attempts initialization on reconnect after a failed prior initialization. |
| src/client/client-load-module.ts | Tracks failed bundle imports and appends s-retry to bust browser module-map caching on retries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for working on this! I tested the proposed fix with a minimal reproduction by blocking the initial lazy-loaded *.entry.js request and then reconnecting the component after restoring the chunk. With this change, the component successfully retries and renders instead of remaining stuck. This looks like a solid improvement for the recovery behavior |
|
We are running into the same issue in our Stencil-based component library. A consumer recently reported this behavior to us, and after debugging we traced it back to this issue. This confirms that the problem is not only reproducible in isolated cases but can also affect real-world consumers of Stencil components. We really appreciate the work the Stencil team is doing. A fix for this would have a meaningful impact for us and other teams building component libraries on top of Stencil. Thanks for looking into this! |
…n hasInitializedComponent The reconnect-retry added in 6514fa6 keyed off `hasInitializedComponent` being unset to decide whether a previous load attempt had failed and should be retried. That flag is also (transiently) unset while an initialization attempt is merely queued/in-flight and hasn't failed at all -- e.g. behind `nextTick` (BUILD.initializeNextTick), or when `connectedCallback` fires again for the same host element before that queued attempt has run. Stencil's own server-side hydration does this deliberately: `serverSideConnected()` (update-component.ts) walks the rendered DOM and re-fires `connectedCallback()` on children regardless of whether they're already mid-initialization. Because the retry condition couldn't tell "failed" apart from "still pending", that second connectedCallback call triggered a premature, duplicate `initializeComponent()` call. The original queued call later ran too (finding `hasInitializedComponent` now set, skipping straight to a second `scheduleUpdate()`), so the component got initialized and rendered twice. This corrupted hydrate output for nested shadow-DOM components -- duplicated vdom annotation comments/text nodes and mismatched `c-id` numbering -- and broke ~30 existing tests across hydrate-shadow-child/-parent/-in-shadow/-no-encapsulation/-slot-fallback in CI (all 4 Unit Tests jobs), while the branch's own new lazy-load-retry.spec.tsx passed, since it doesn't exercise hydration. Root cause confirmed locally by instrumenting initializeComponent() to count/log invocations per hostRef: the duplicate call's stack traced directly to the `nextTick(() => initializeComponent(...))` callback in connected-callback.ts, with hasInitializedComponent already true and $lazyInstance$ still unset at that point. Fix: add a dedicated HOST_FLAGS.hasFailedLoad bit that is only set when a lazy bundle's dynamic import() genuinely fails to resolve a constructor, and only cleared when a fresh attempt starts. The reconnect-retry in connected-callback.ts now checks that flag instead of the absence of hasInitializedComponent, so it no longer fires for attempts that are simply still in flight. Verified: the previously-failing hydrate-shadow-child/-parent/ -in-shadow/-no-encapsulation/-slot-fallback suites and the new lazy-load-retry suite all pass, and the full `npm run test.jest -- --runInBand` suite is green (4085/4102 passed, 17 skipped, 0 failed).
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/runtime/connected-callback.ts:137
- The retry path calls
initializeComponentsynchronously even whenBUILD.initializeNextTickis enabled. On first connect, Stencil defers initialization withnextTickto accommodate frameworks that set attributes afterconnectedCallback(e.g. Angular); the retry should preserve the same ordering semantics to avoid reintroducing the original timing issues on reconnect.
initializeComponent(elm, hostRef, cmpMeta);
Contributor
|
thanks for your work on this @yasinkocak - I've been on vacation and will look at this properly at some point this week ❤️ |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/runtime/connected-callback.ts:137
- In the retry path (hasFailedLoad on reconnect), initialization is triggered synchronously even when
BUILD.initializeNextTickis enabled. That makes retry initialization behave differently from the first-connect path (which defers tonextTickto accommodate frameworks like Angular setting attributes afterconnectedCallback). To keep behavior consistent and avoid subtle timing regressions, the retry should honorBUILD.initializeNextTickas well.
initializeComponent(elm, hostRef, cmpMeta);
Comment on lines
71
to
77
| (importedModule) => { | ||
| failedLoadAttempts.delete(bundleId); | ||
| if (!BUILD.hotModuleReplacement) { | ||
| cmpModules.set(bundleId, importedModule); | ||
| } | ||
| return importedModule[exportName]; | ||
| }, |
johnjenkins
requested changes
Aug 5, 2026
johnjenkins
left a comment
johnjenkins
left a comment
Contributor
There was a problem hiding this comment.
great work - tsym for looking into this 🙏
General comments:
- Retry has no cap or backoff atm so will just retry forever. Suggest a cap of 3 with a 1s interval between retries - wdyt?
- To complete the "component can now recover" story, can you look at
componentOnReady. Atm$onReadyResolve$(elm)is called unconditionally in initializeComponent's catch block on the first failure (in initialize-component.ts 212-214), and$onReadyPromise$is a plain Promise created once per host element inregisterHost()(client-host-ref.ts 62) so can only settle once. Any caller that awaitedcomponentOnReady()before the retry succeeds will have already received the (un-hydrated) element and will never be notified when the retry actually works.
Comment on lines
+8
to
+15
| /** | ||
| * Tracks how many times the dynamic `import()` for a given lazy bundle has | ||
| * failed. A failed dynamic import is cached by the browser's own module map | ||
| * for the resolved specifier URL, so a bare retry of the exact same URL | ||
| * would be rejected immediately without ever reaching the network again. | ||
| * This lets a retry (see `connected-callback.ts`) bust that cache with a | ||
| * query param, the same way `hmrVersionId` already does for HMR. | ||
| */ |
Contributor
There was a problem hiding this comment.
quite verbose :)
Suggested change
| /** | |
| * Tracks how many times the dynamic `import()` for a given lazy bundle has | |
| * failed. A failed dynamic import is cached by the browser's own module map | |
| * for the resolved specifier URL, so a bare retry of the exact same URL | |
| * would be rejected immediately without ever reaching the network again. | |
| * This lets a retry (see `connected-callback.ts`) bust that cache with a | |
| * query param, the same way `hmrVersionId` already does for HMR. | |
| */ | |
| /** | |
| * Tracks how many times a dynamic `import()` for a given lazy bundle has failed. | |
| * Used to cache bust the retry attempt in connected-callback.ts | |
| */ |
Comment on lines
+7
to
+12
| /** | ||
| * Regression tests for a bug where a host element would be permanently | ||
| * "bricked" (never rendered, no lifecycle callbacks) if its lazy bundle | ||
| * failed to load a single time (e.g. a dropped network request). See: | ||
| * https://github.com/stenciljs/core/issues/6771 | ||
| */ |
Contributor
There was a problem hiding this comment.
unnecessary :)
Suggested change
| /** | |
| * Regression tests for a bug where a host element would be permanently | |
| * "bricked" (never rendered, no lifecycle callbacks) if its lazy bundle | |
| * failed to load a single time (e.g. a dropped network request). See: | |
| * https://github.com/stenciljs/core/issues/6771 | |
| */ |
Comment on lines
+122
to
+136
| // A previous initialization attempt for this host element failed | ||
| // (e.g. the lazy bundle's dynamic import() was rejected) -- see | ||
| // `initialize-component.ts`. Retry now that we're reconnecting, | ||
| // rather than leaving the element permanently un-upgraded for the | ||
| // rest of the page's lifetime. | ||
| // | ||
| // This must key off the dedicated `hasFailedLoad` flag rather than | ||
| // `hasInitializedComponent` being unset: that flag is also | ||
| // (transiently) unset while an initialization attempt is merely | ||
| // queued/in-flight and hasn't failed at all -- e.g. behind | ||
| // `nextTick`, or when `connectedCallback` fires again for the same | ||
| // element before that queued attempt has run, which Stencil's own | ||
| // server-side hydration deliberately does (see | ||
| // `serverSideConnected()` in `update-component.ts`). Retrying in | ||
| // that case double-initializes the component. |
Contributor
There was a problem hiding this comment.
again - very verbose and repeated in constants.ts
Suggested change
| // A previous initialization attempt for this host element failed | |
| // (e.g. the lazy bundle's dynamic import() was rejected) -- see | |
| // `initialize-component.ts`. Retry now that we're reconnecting, | |
| // rather than leaving the element permanently un-upgraded for the | |
| // rest of the page's lifetime. | |
| // | |
| // This must key off the dedicated `hasFailedLoad` flag rather than | |
| // `hasInitializedComponent` being unset: that flag is also | |
| // (transiently) unset while an initialization attempt is merely | |
| // queued/in-flight and hasn't failed at all -- e.g. behind | |
| // `nextTick`, or when `connectedCallback` fires again for the same | |
| // element before that queued attempt has run, which Stencil's own | |
| // server-side hydration deliberately does (see | |
| // `serverSideConnected()` in `update-component.ts`). Retrying in | |
| // that case double-initializes the component. | |
| // init for this host element failed. Retry now that we're reconnecting. |
Comment on lines
+58
to
+67
| // The lazy bundle failed to load (e.g. a dropped network request). | ||
| // Clear the "initialized" flag and mark the load as failed so a | ||
| // future `connectedCallback` (see `connected-callback.ts`) is able | ||
| // to retry loading this component instead of leaving the host | ||
| // element permanently un-upgraded. `hasFailedLoad` (rather than | ||
| // just the absence of `hasInitializedComponent`) is what gates the | ||
| // retry, since `hasInitializedComponent` is also unset while an | ||
| // attempt is merely queued/in-flight (e.g. behind `nextTick`, or | ||
| // between repeated `connectedCallback` invocations during | ||
| // server-side hydration) -- neither of which is a failure. |
Contributor
There was a problem hiding this comment.
again - unnecessary - I think it's clear from context :)
Suggested change
| // The lazy bundle failed to load (e.g. a dropped network request). | |
| // Clear the "initialized" flag and mark the load as failed so a | |
| // future `connectedCallback` (see `connected-callback.ts`) is able | |
| // to retry loading this component instead of leaving the host | |
| // element permanently un-upgraded. `hasFailedLoad` (rather than | |
| // just the absence of `hasInitializedComponent`) is what gates the | |
| // retry, since `hasInitializedComponent` is also unset while an | |
| // attempt is merely queued/in-flight (e.g. behind `nextTick`, or | |
| // between repeated `connectedCallback` invocations during | |
| // server-side hydration) -- neither of which is a failure. | |
| // The lazy bundle failed to load (e.g. a dropped network request). |
Comment on lines
+59
to
+65
| /** | ||
| * Set when a lazy component's dynamic `import()` fails to resolve a | ||
| * constructor. Distinct from `hasInitializedComponent` being unset, which | ||
| * is also (transiently) true while an initialization attempt is merely | ||
| * queued/in-flight (e.g. behind `nextTick`) and hasn't failed at all. | ||
| * Only this flag should gate a `connectedCallback` retry. | ||
| */ |
Contributor
There was a problem hiding this comment.
Suggested change
| /** | |
| * Set when a lazy component's dynamic `import()` fails to resolve a | |
| * constructor. Distinct from `hasInitializedComponent` being unset, which | |
| * is also (transiently) true while an initialization attempt is merely | |
| * queued/in-flight (e.g. behind `nextTick`) and hasn't failed at all. | |
| * Only this flag should gate a `connectedCallback` retry. | |
| */ | |
| /** | |
| * Set when a lazy component's dynamic `import()` fails to resolve a a | |
| * constructor. Distinct from `hasInitializedComponent` being unset, which | |
| * is true while an initialization attempt is merely queued/in-flight. | |
| */ |
Comment on lines
+37
to
42
| // Starting a fresh attempt clears any failure recorded by a previous one. | ||
| hostRef.$flags$ &= ~HOST_FLAGS.hasFailedLoad; | ||
|
|
||
| const bundleId = cmpMeta.$lazyBundleId$; | ||
| if (BUILD.lazyLoad && bundleId) { | ||
| // lazy loaded components |
Contributor
There was a problem hiding this comment.
nit .. but this only pertains to lazy bundles so move into the lazyLoad gated block
Suggested change
| const bundleId = cmpMeta.$lazyBundleId$; | |
| if (BUILD.lazyLoad && bundleId) { | |
| // lazy loaded components | |
| // clear any failure recorded by a previous attempt | |
| hostRef.$flags$ &= ~HOST_FLAGS.hasFailedLoad; |
Contributor
There was a problem hiding this comment.
there doesn't appear to be any coverage here on the actual network-facing fix(?) - the ?s-retry=N query-param logic (because you're using the testing platform's mock loadModule).
We probably need a more focused unit test; something to guard against regressions on the cacheBustParams join logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the current behavior?
GitHub Issue Number: #6771
A host element is permanently "bricked" for the rest of the page's lifetime if its lazy bundle's dynamic
import()fails a single time (dropped network request, flaky mobile connection, ad-blocker, stale CDN entry, etc.) — no rendered content, no lifecycle callbacks, and no recovery, not even for a brand-new element of the same tag, until a full page reload.Root cause:
HOST_FLAGS.hasInitializedComponentis set before the load attempt ininitialize-component.tsand never cleared on failure.HOST_FLAGS.hasConnected(set unconditionally on first connect) makesconnected-callback.tsskip re-initialization on every subsequent reconnect.import()specifier, so even a bare retry of the exact same bundle URL never reaches the network again — confirmed in a live-browser repro (see Testing below).What is the new behavior?
HOST_FLAGS.hasInitializedComponentis cleared instead of staying stuck.connected-callback.tsretriesinitializeComponent()the next time the host element is reconnected (disconnect + reconnect), instead of only handling the "already has a$lazyInstance$" case.client-load-module.tsappends a cache-busting query param (?s-retry=N) on retries, the same pattern already used for?s-hmr=, so the retry actually reaches the network instead of being short-circuited by the browser's module map.No public API changes. No behavior change on the happy path — a retry only ever happens after a load has actually failed.
Documentation
N/A — this is an internal runtime resilience fix, no public API or documented behavior changes.
Does this introduce a breaking change?
Testing
src/runtime/test/lazy-load-retry.spec.tsx(2 new tests): verifieshasInitializedComponentis cleared on a failed load, and that a disconnect+reconnect after a failure re-attempts initialization and succeeds once the module becomes available.npm run test.jest -- --runInBand, 64 suites / 651 tests acrosssrc/runtime+src/client) passes with no regressions.*.entry.jsrequest to simulate a blocked/flaky network request):Constructor for "web-button#undefined" was not found, element stays un-hydrated (empty shadow root).GET /build/p-xxx.entry.js?s-retry=1 → 200, element renders correctly (class="hydrated", populated shadow DOM) — all without a page reload.Other information
Per CONTRIBUTING.md, issue #6771 was filed first with a full reproducible write-up before starting this PR.