Skip to content

fix: stop unrelated third-party JS errors from blocking init REQ-6294 - #28

Open
souhaibparcellab wants to merge 2 commits into
mainfrom
fix/REQ-6294-third-party-error-blocks-init
Open

fix: stop unrelated third-party JS errors from blocking init REQ-6294#28
souhaibparcellab wants to merge 2 commits into
mainfrom
fix/REQ-6294-third-party-error-blocks-init

Conversation

@souhaibparcellab

Copy link
Copy Markdown

Related Ticket

https://parcellab.atlassian.net/browse/REQ-6294

The bug

The React and Vue loaders registered a global window error listener for the whole duration of the parcelLab bundle download, and treated whatever it caught as a parcelLab load failure:

window.addEventListener("error", onScriptError);   // fires for ANY page error
container.appendChild(scriptEl);
...
function onLoadComplete() {
  cleanup();
  if (err) { reject(err); }   // <- unrelated error kills init

window's error event fires for every uncaught exception anywhere on the host page. On unisport.pl, Bazaarvoice threw not configured for the domain, our bundle then loaded perfectly well, and the loader threw it away — so initialize() was never called and Track & Trace never rendered. Nothing compared the event against our script.

Unisport is just the visible victim. Any customer running a third-party script that throws — analytics, chat, session replay, ad tags, A/B testing, cookie banners — is exposed to the same latent failure.

The fix

Delete the listener and lean on a signal that is already precise.

The plugin bundle publishes its global as its final statement (Object.defineProperty(window, "parcelLabTrackAndTrace", …)). A script that throws mid-execution still fires load, so the global being absent means our code failed — and nothing on the host page can cause it. v5 already had this check sitting unused next to the buggy one; v3 had none and now does.

Also fixed on the same path

Timeout never rejected The 15s timer only recorded an error, so a bundle that never loaded left the promise pending forever. It now settles — and resolves if the global did turn up in the meantime.
Missing return after reject Fell through to resolve(). Harmless today (settle-once) but it hid intent.
React double-init The effect keyed on [tntRef.current], which flips from undefined to the element after first render — so any re-render appended a second <script> and called initialize() twice. Now guarded to once per mount, which also covers StrictMode's double invocation.
Vue shipped a dev build The committed Vue artifacts were webpack development bundles using eval(), which a strict CSP blocks outright. Rebuilt as production bundles — v5/vue/index.js drops from 16.7 kB to 7.5 kB.

Scope

All four loaders had the bug, not just the one named in the ticket: v3 and v5, React and Vue.

Tests

44 tests (vitest + jsdom), 19 of which fail against the previous code — so they genuinely pin the regression rather than just describing the new behaviour.

The suite covers, per loader: init survives an unrelated third-party error; no global error listener is registered at all; genuine failures (bundle loaded but global missing, script 404, timeout) still surface; bootstrap happens exactly once.

Because v3/ and v5/ are committed build artifacts that npm publishes, the tests also run against the built output, not just src/. And CI now rebuilds and fails if the committed artifacts have drifted from src/ — this repo's easiest mistake is fixing src/ and shipping nothing.

Reviewer notes

  • The diff is large mostly because of package-lock.json and the regenerated Vue bundles. The behavioural change is the four files in src/.
  • This repo has no npm release automation. Merging only redeploys the web component to the CDN; the React/Vue packages go to GitHub Packages via a manual npm publish. So merging this does not ship it to customers — someone has to publish. The published 1.0.6 dates from 2026-01-05 and main has moved since, so it's worth diffing before publishing.

Follow-ups, deliberately not in this PR

  • react/react-dom/vue are peerDependencies without peerDependenciesMeta.optional, so since npm 7 a React-only customer gets Vue 3 installed for nothing.
  • Multiple mounted instances still each inject their own <script>/<link>; no cross-instance dedupe.
  • AC Configure Renovate #1 on the ticket (bootstrap telemetry) is not addressed — it needs a collection endpoint decision first, since a failed bootstrap means no plugin and therefore no existing reporting channel.

Test plan

  • npm test — 44 passing
  • Verified 19 tests fail when the sources are reverted to main
  • Verified the build is byte-reproducible across runs, so the CI drift check is stable
  • Confirmed no onScriptError and no eval( remain in any published artifact
  • Smoke-test a real React storefront against a prerelease before publishing

The React and Vue loaders registered a global `window` error listener while
the parcelLab bundle was downloading, and treated anything it caught as a
parcelLab load failure. Because that event fires for every uncaught error on
the host page, an unrelated third-party script throwing during the download
window made the loader skip `initialize()` entirely and the widget never
rendered.

This is what happened on unisport.pl: Bazaarvoice threw "not configured for
the domain", our bundle then loaded perfectly well, and the loader discarded
it. Any customer running a third-party script that throws is exposed.

The listener is replaced with a check for the global the bundle publishes as
its final statement. A script that throws while executing still fires `load`,
so the absence of that global is a precise signal that *our* script failed,
and nothing else on the page can trigger it.

Also fixed in the same path:

- The 15s timeout only recorded an error and never rejected, so a bundle that
  never loaded left the promise pending forever. It now settles, and resolves
  if the global did appear in the meantime.
- `reject()` was not followed by `return`, falling through to `resolve()`.
- React bootstrapped off `[tntRef.current]`, which changes from undefined to
  the element after first render, so any re-render appended a second script
  and initialised twice. Now guarded to run once per mount.
- The committed Vue artifacts were webpack *development* builds using eval(),
  which a strict CSP blocks outright. Rebuilt as production bundles;
  v5/vue/index.js drops from 16.7 kB to 7.5 kB.

All four loaders (v3 and v5, React and Vue) carried the same bug. v3 had no
global check at all, so one was added.

Adds vitest + jsdom coverage: 44 tests, of which 19 fail against the previous
code. Tests run against the committed build artifacts as well as src/, and CI
now rebuilds and fails if the two have drifted.
@souhaibparcellab
souhaibparcellab requested a review from a team as a code owner July 28, 2026 11:15
@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

vitest 4 imports styleText from node:util, which requires Node 20+. The
deploy workflow's Node 18 stays as-is; it only runs AWS CLI steps.
@Dariia
Dariia self-requested a review July 28, 2026 12:22
@Dariia Dariia self-assigned this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants