Skip to content

WIP 5944 Fix race condition: Add event listeners to DOM elements before mounting them #5968

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

Closed
wants to merge 1 commit into from

Conversation

Phaqui
Copy link
Contributor

@Phaqui Phaqui commented Feb 7, 2021

This is a WIP on #5944 - Trying to fix a race condition where a svelte:head tag containing a script with a src attribute doesn't have it's onload handler attached before after the element is added to the DOM, resulting in a race condition, where if the browser finishes getting the script before the onload listener is attached, the onload event handler will not run, see the Issue for more information about the bug.

So far, it fixes the bug in question by rewriting code generation for the "mount" function of Fragments so that attaching event listeners comes before appending the elements to the DOM.

Help needed! Currently, this breaks the test "deconflict-contextual-action".
As far as I can tell, the test seems to rely on the current order...? I don't know why. Maybe switching this order would break something else. Maybe there are cases where the elements have to be added to the DOM before the event listeners are attached (maybe in order to prevent an event handler to fire at an incorrect time?)... I am lost.

So, bottom line: I don't know why the mount code is being generated in this order. Does it matter, for reasons that I cannot see?

  • If that is the case, which other ways could we solve this issue?
  • If that is NOT the case, can the failing test be rewritten?

@stale
Copy link

stale bot commented Sep 5, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Sep 5, 2021
@stale
Copy link

stale bot commented Sep 30, 2021

This pull request has been closed as it was previously marked as stale and saw no subsequent activity.

@stale stale bot closed this Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant