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

potentially fix monaco editor build bug #9690

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Conversation

jwunderl
Copy link
Member

@jwunderl jwunderl commented Sep 19, 2023

I think this will fix the bug that rare build issue that caused microsoft/pxt-arcade#6077. The bits leading me to think that are that it's pointing at /codicons/codicon/codicon.ttf while grabbing the editor.css, which matches the error we got
image
and that inlineCodiconFont's job is to replace that relative path with an inline font - so the replace in that fn is not being applied to the end result (it's just a few lines above this change). It's a bit hard to induce the bug, but I think it's a race condition with the fs.readFileSync vs gulp.src in inlineCodiconFont and copyMonacoEditor functions, where it normally runs fine but very rare fs lag causes inlineCodiconFont to get applied first then overwritten. I guess I can try inducing it by throwing in a for (let i = 0; i < 10000000000; i++) {i ++} or something in copyMonacoEditor?

I did a local serve and uploadtrg and both have icons as expected, it also just makes sense that the mutation of the file should not be running parallel to copying the file~

(I also kinda wonder if this was the cause for comment on line 504 -- it's fetching from the cdn relative to the .css file, but the path at the end is just for reference / the hash is what matters, so it was always pulling in the css and trying to use it as ttf. Probably the right fix though as would maybe be a bit of a headache to replace in the right cdn path)

@jwunderl jwunderl requested review from riknoll and a team September 19, 2023 21:47
@jwunderl jwunderl merged commit b2752a7 into master Sep 26, 2023
7 checks passed
@jwunderl jwunderl deleted the potentiallyFixBlueMoonBug branch September 26, 2023 21:21
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