-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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(ext/node): fix prismjs compatibiliy in Web Worker #25062
Conversation
ext/node/global.rs
Outdated
@@ -80,7 +81,7 @@ const MANAGED_GLOBALS: [&[u16]; 13] = [ | |||
]; | |||
|
|||
const SHORTEST_MANAGED_GLOBAL: usize = 4; | |||
const LONGEST_MANAGED_GLOBAL: usize = 14; | |||
const LONGEST_MANAGED_GLOBAL: usize = 17; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these could be calculated in a const expr so they can't fall out of date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could that be easily/reasonably done? Looked into const fn
feature, but it seems very limited..
Anyway this value is indirectly tested via the test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const MANAGED_GLOBAL_INFO: (usize, usize) = {
let mut longest = MANAGED_GLOBALS[0].len();
let mut shortest = MANAGED_GLOBALS[0].len();
let mut i = 0;
while i < MANAGED_GLOBALS.len() {
let g = MANAGED_GLOBALS[i];
if g.len() > longest {
longest = g.len();
}
if g.len() < shortest {
shortest = g.len();
}
i += 1;
}
(shortest, longest)
};
const SHORTEST_MANAGED_GLOBAL: usize = MANAGED_GLOBAL_INFO.0;
const LONGEST_MANAGED_GLOBAL: usize = MANAGED_GLOBAL_INFO.1;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PrismJS uses
WorkerGlobalScope
andself
for detecting browser's Web Worker context:https://github.com/PrismJS/prism/blob/59e5a3471377057de1f401ba38337aca27b80e03/prism.js#L11
Now the detection logic above is broken when it's imported from Deno's Web Worker context because we only hide
self
(Prism assumes whenWorkerGlobalScope
is available,self
is also available).This PR fixes the above by also hiding
WorkerGlobalScope
global in Node compat mode.closes #25008