Skip to content

Fix -sDETERMINISTIC under node >= 16 #19663

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

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Fix -sDETERMINISTIC under node >= 16 #19663

merged 1 commit into from
Jun 21, 2023

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jun 21, 2023

It turns out you can't override performance.now on modern versions of not. Not sure if that is bug, but we have to work around it.

This should make the CI green again since we updated emsdk to node 16.

@sbc100 sbc100 force-pushed the deterministic branch 2 times, most recently from 3cdd7b9 to c5ef01d Compare June 21, 2023 01:07
@sbc100 sbc100 requested a review from kripken June 21, 2023 01:08
It turns out you can't override `performance.now` on modern versions
of not.  Not sure if that is bug, but we have to work around it.

This should make the CI green again since we updated emsdk to node 16.

// Setting performance.now to deterministicNow doesn't work so we instead
// use a helper function in parseTools (getPerformanceNow()) to call it
// directly.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe link to this PR as a source for the issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure thats really necessary.. isn't that what git blame is for?

@sbc100 sbc100 merged commit 4158554 into main Jun 21, 2023
@sbc100 sbc100 deleted the deterministic branch June 21, 2023 02:31
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