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

Mock modules in tests using Module #28678

Merged
merged 1 commit into from
Mar 29, 2024
Merged

Conversation

yungsters
Copy link
Contributor

Summary

Fixes a type validation error introduced in newer versions of Node.js when calling Module.prototype._compile in our unit tests. (I tried but have yet to pinpoint the precise change in Node.js that introduced this vaildation.)

The specific error that currently occurs when running unit tests with Node.js v18.16.1:

TypeError: The "mod" argument must be an instance of Module. Received an instance of Object

      80 |
      81 |     if (useServer) {
    > 82 |       originalCompile.apply(this, arguments);
         |                       ^
      83 |
      84 |       const moduleId: string = (url.pathToFileURL(filename).href: any);
      85 |

This fixes the type validation error by mocking modules using new Module() instead of plain objects.

How did you test this change?

Ran the unit tests successfully:

$ node --version
v18.16.1

$ yarn test

@react-sizebot
Copy link

Comparing: 425f72b...84446de

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 177.14 kB 177.14 kB = 55.06 kB 55.06 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 172.96 kB 172.96 kB = 53.92 kB 53.92 kB
facebook-www/ReactDOM-prod.classic.js = 592.88 kB 592.88 kB = 104.01 kB 104.01 kB
facebook-www/ReactDOM-prod.modern.js = 574.57 kB 574.57 kB = 101.03 kB 101.03 kB
test_utils/ReactAllWarnings.js Deleted 65.18 kB 0.00 kB Deleted 16.30 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 65.18 kB 0.00 kB Deleted 16.30 kB 0.00 kB

Generated by 🚫 dangerJS against 84446de

Copy link
Contributor

@jackpope jackpope left a comment

Choose a reason for hiding this comment

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

thank you!

@yungsters yungsters merged commit 13f3543 into facebook:main Mar 29, 2024
38 checks passed
@yungsters yungsters deleted the test-mod-error branch March 29, 2024 20:19
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
## Summary

Fixes a type validation error introduced in newer versions of Node.js
when calling `Module.prototype._compile` in our unit tests. (I tried but
have yet to pinpoint the precise change in Node.js that introduced this
vaildation.)

The specific error that currently occurs when running unit tests with
Node.js v18.16.1:

```
TypeError: The "mod" argument must be an instance of Module. Received an instance of Object

      80 |
      81 |     if (useServer) {
    > 82 |       originalCompile.apply(this, arguments);
         |                       ^
      83 |
      84 |       const moduleId: string = (url.pathToFileURL(filename).href: any);
      85 |
```

This fixes the type validation error by mocking modules using `new
Module()` instead of plain objects.

## How did you test this change?

Ran the unit tests successfully:

```
$ node --version
v18.16.1

$ yarn test
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants