Skip to content

Fix sequential script loading in webtrees.load()#5359

Open
magicsunday wants to merge 1 commit intofisharebest:mainfrom
magicsunday:fix/sequential-script-loading
Open

Fix sequential script loading in webtrees.load()#5359
magicsunday wants to merge 1 commit intofisharebest:mainfrom
magicsunday:fix/sequential-script-loading

Conversation

@magicsunday
Copy link
Copy Markdown
Contributor

Summary

  • Fix race condition in webtrees.load() where inline scripts execute before external scripts have finished loading
  • Replace forEach with a Promise chain that waits for each external script's onload event before appending the next script

Problem

The webtrees.load() function (which replaced jQuery's .load()) dynamically injects <script> elements via appendChild. External scripts (with src) load asynchronously by default, but the forEach loop immediately appends the next script without waiting. This causes inline scripts that depend on a preceding external library to fail with errors like:

Uncaught TypeError: WebtreesFanChart.FanChart is not a constructor

This affects any custom module that uses the common pattern of pushing an external script tag followed by an inline initialization script via View::push('javascript').

Fix

Scripts are now loaded sequentially using a Promise chain. External scripts wait for their onload event before the next script in the chain is appended to the DOM.

Test plan

  • Custom modules using View::push('javascript') with separate external and inline script blocks load correctly
  • Tested with magicsunday/webtrees-fan-chart, webtrees-pedigree-chart, and webtrees-descendants-chart

The webtrees.load() function injects scripts dynamically via
appendChild, but external scripts (with src) load asynchronously.
Subsequent inline scripts execute immediately before the external
script has finished loading, causing "not a constructor" errors
for any module that uses separate script tags for library loading
and initialization.

Replace forEach with a Promise chain that waits for each external
script's onload event before appending the next script element.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 35.37%. Comparing base (c338276) to head (ac31a62).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #5359   +/-   ##
=========================================
  Coverage     35.37%   35.37%           
  Complexity    11196    11196           
=========================================
  Files          1166     1166           
  Lines         48031    48031           
=========================================
  Hits          16990    16990           
  Misses        31041    31041           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@magicsunday
Copy link
Copy Markdown
Contributor Author

For reference, this regression was introduced in commit a3ca8e1 ("Convert jQuery code to VanillaJS"). The previous jQuery .load() implementation executed scripts sequentially, while the vanilla JS replacement uses forEach + appendChild which starts external script downloads without waiting for onload before appending the next script.

This affects all custom modules that use the common two-block pattern (View::push('javascript') with an external script followed by inline initialization).

magicsunday added a commit to magicsunday/webtrees-fan-chart that referenced this pull request Apr 10, 2026
Replace two separate View::push('javascript') blocks with a single
<script type="module"> block using dynamic import(). The import()
promise waits for the external script to load before initializing,
avoiding the race condition in webtrees.load() (PR fisharebest/webtrees#5359).
magicsunday added a commit to magicsunday/webtrees-fan-chart that referenced this pull request Apr 10, 2026
Replace two separate View::push('javascript') blocks with a single
<script type="module"> block using dynamic import(). The import()
promise waits for the external script to load before initializing,
avoiding the race condition in webtrees.load() (PR fisharebest/webtrees#5359).
@magicsunday
Copy link
Copy Markdown
Contributor Author

Our modules no longer trigger this race condition — we switched to dynamic import().then() inside a single <script type="module">.

Keeping this PR open since the fix in webtrees.load() would still benefit other custom modules that use external script tags via View::push('javascript'). Happy to rebase if needed.

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.

1 participant