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

test-perf: Load Benckmark.js first in Node.js #3008

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

kfule
Copy link
Contributor

@kfule kfule commented Feb 14, 2025

Description

Since Node21, global.navigator has been implemented, and together with browserMock, Benchmark.js incorrectly identifies the execution environment as a browser. It causes error on npm run perf in newer nodejs.
If Benchmark.js is loaded before browserMock, the misidentification will be avoided.

Motivation and Context

I have run npm run perf in some environments and found that the newer nodejs versions have an error.
The problem seemed to be caused by benchmark.js misidentifying the execution environment as a browser.

How Has This Been Tested?

I ran npm run perf on node22 and confirmed that no errors occurred.
Of course, node 18 and 20 do not cause errors. Also, benchmark results should not be affected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My change requires a documentation update, and I've opened a pull request to update it already:
  • I have read https://mithril.js.org/contributing.html.

Since Node21, global.navigator has been implemented, and together with browserMock, Benchmark.js incorrectly identifies the execution environment as a browser. If Benchmark.js is loaded before browserMock, the misidentification will be avoided.
@kfule kfule requested a review from a team as a code owner February 14, 2025 13:27
@dead-claudia dead-claudia merged commit 5608a71 into MithrilJS:main Feb 15, 2025
7 checks passed
@JAForbes JAForbes mentioned this pull request Feb 15, 2025
@kfule kfule deleted the node22_test-perf branch February 15, 2025 14:13
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