Skip to content
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

Fix #1946 - Do not hold while bootstrapping #1953

Merged
merged 1 commit into from Jan 26, 2024

Conversation

WebReflection
Copy link
Contributor

Description

This MR fixes #1946 by not holding while looping to activate editors.

Here a basic, pseudo code, representation of the issue that is solved:

async function init() {
  for (const script of document.querySelectorAll("script[type='editor']")) {
    // here the script type is changed to avoid being part of this loop
    script.type += '-activated';
    // but if in here we hold anything async ...
    await bootstrap(script);
    // the loop will execute before giving a chance to other, already discovered,
    // scripts to initialize themselves and get a new `type`
  }
}

// one call ASAP
init();

// MutationObserver calls triggered by first script bootstrap
init();

// any other reason to call init again
init();

As summary, this MR fixes in one single loop the type and queue bootstraps instead of waiting per each of them so that even if invoked multiple consecutive times, each script is guaranteed to be bootstrapped only once.

Changes

  • use a queue instead of holding on every init call
  • create a smoke-test that present no issue whatsoever with or without cache enabled

Checklist

  • All tests pass locally
  • I have updated CHANGELOG.md
  • I have created documentation for this(if applicable)

@WebReflection
Copy link
Contributor Author

OK, in retrospective, I am not 100% sure why the previous code would've ever bootstrapped same node twice, as the script.type is clearly assigned and it should never be looped over again ... however this MR guarantees the order of nodes matter and that the bootstrap is incremental, not all at once ... can anyone please confirm we don't have issues anymore in here? I start thinking maybe the issue was the ID assignment or something, not the exact order / timing of the bootstrap 🤔

This MR fixes both but as I could never reproduce this issue, extra eyes might be welcome, thanks!

@WebReflection
Copy link
Contributor Author

This has been confirmed as solved.

/cc @ntoll or anyone else happy to approve?

Copy link
Member

@ntoll ntoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@WebReflection WebReflection merged commit 63f2453 into pyscript:main Jan 26, 2024
3 checks passed
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.

the py-editor is created repeatedly
2 participants