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: ensure manager entries load even if preceding ones failed #20286

Merged
merged 3 commits into from
Dec 15, 2022

Conversation

ndelangen
Copy link
Member

Issue: #20285

What I did

I changed the generated code loading managerEntries so there's error handling

@ndelangen ndelangen self-assigned this Dec 15, 2022
@ndelangen ndelangen requested a review from yannbf December 15, 2022 10:58
@ndelangen
Copy link
Member Author

ndelangen commented Dec 15, 2022

So this doesn't work because of a race-condition between the manager self-executing and managerEntries self-injecting themselves into the manager runtime.

If the managerEntires inject themselves too late, the manager won't pick them up, meaning the addons are missing.
This race-condition might actually already kinda exist, so something that might be happening now, but by making the import imperative, it's now guaranteed that the manager has already fully mounted and called provider.getElements.

It will only do this once, and so: inject too late, and it will never ever show up.

I need to look into how to fix this, but it might require a drastic change in the manager-api. (allowing panels to be added async, after mounting)

getElements: (type) => provider.getElements(type),

@ndelangen
Copy link
Member Author

It all being async would also make it difficult to preserve the order of addons being injected.

@ndelangen ndelangen requested a review from shilman December 15, 2022 15:36
@ndelangen
Copy link
Member Author

OK, different approach!

I add a try { <<code >> } catch (e) { .. } around each managerEntry inside of esbuild.

@ndelangen
Copy link
Member Author

@shilman self-merging for release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant