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

"Cannot find module 'msw/node'" in Jest JSDOM environment #1786

Closed
4 tasks done
textbook opened this issue Oct 23, 2023 · 24 comments
Closed
4 tasks done

"Cannot find module 'msw/node'" in Jest JSDOM environment #1786

textbook opened this issue Oct 23, 2023 · 24 comments
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node

Comments

@textbook
Copy link

textbook commented Oct 23, 2023

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 18 or higher

Node.js version

v18.18.2

Reproduction repository

https://github.com/textbook/msw2-mre

Reproduction steps

npm ci && npm test

Current behavior

Error message

Cannot find module 'msw/node' from 'path/to/file'

$ npm test

> msw2@0.1.0 test
> jest

 FAIL  ./index.test.js
  ● Test suite failed to run

    Cannot find module 'msw/node' from 'index.test.js'

      1 | const { http, HttpResponse } = require("msw");
    > 2 | const { setupServer } = require("msw/node");
        |                         ^
      3 |
      4 | const server = setupServer();
      5 |

      at Resolver._throwModNotFoundError (node_modules/jest-resolve/build/resolver.js:427:11)
      at Object.require (index.test.js:2:25)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.961 s
Ran all test suites.
Error: Process completed with exit code 1.

🔴 Failing Actions run

Expected behavior

The test should pass, as it does if you switch to the Node environment by updating jest.config.js as follows:

diff --git a/jest.config.js b/jest.config.js
index f343a5c..4e07134 100644
--- a/jest.config.js
+++ b/jest.config.js
@@ -13,5 +13,5 @@ module.exports = {
                ReadableStream,
                TextEncoder,
        },
-       testEnvironment: "jsdom",
+       testEnvironment: "node",
 };

Outcome:

$  npm test

> msw2@0.1.0 test
> jest

 PASS  ./index.test.js
  ✓ works (10 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.203 s
Ran all test suites.
@textbook textbook added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node labels Oct 23, 2023
@textbook
Copy link
Author

textbook commented Oct 23, 2023

  • It's possibly related to Module not found: Package path ./node is not exported from package ./node_modules/msw #1705, but it's a slightly different error message and I'm not using Next.js.
  • I also tried using setupWorker from msw/browser, in case the JSDOM environment was sufficiently browser-y for that to work, but got Invariant Violation: [MSW] Failed to execute `setupWorker` in a non-browser environment. Consider using `setupServer` for Node.js environment instead.

@csantos-nydig
Copy link

I got a similar error but running ESLINT

error: Resolve error: Package subpath './browser' is not defined by "exports" in /Users/cesar.santos/app/node_modules/msw/package.json
    at new NodeError (node:internal/errors:393:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:358:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:612:7)
    at resolveExports (node:internal/modules/cjs/loader:522:36)
    at Module._findPath (node:internal/modules/cjs/loader:562:31)
    at findModulePath (/Users/cesar.santos/app/node_modules/eslint-import-resolver-alias/index.js:99:27)
    at exports.resolve (/Users/cesar.santos/app/node_modules/eslint-import-resolver-alias/index.js:75:10)
    at v2 (/Users/cesar.santos/app/node_modules/eslint-module-utils/resolve.js:116:23)
    at withResolver (/Users/cesar.santos/app/node_modules/eslint-module-utils/resolve.js:121:14)
    at fullResolve (/Users/cesar.santos/app/node_modules/eslint-module-utils/resolve.js:138:22)
    at relative (/Users/cesar.santos/app/node_modules/eslint-module-utils/resolve.js:83:10)
    at resolve (/Users/cesar.santos/app/node_modules/eslint-module-utils/resolve.js:219:12)
    at isImportingSelf (/Users/cesar.santos/app/node_modules/eslint-plugin-import/lib/rules/no-self-import.js:14:70)
    at commonjs (/Users/cesar.santos/app/node_modules/eslint-plugin-import/lib/rules/no-self-import.js:35:9)
    at checkSourceValue (/Users/cesar.santos/app/node_modules/eslint-module-utils/moduleVisitor.js:29:5)
    at checkCommon (/Users/cesar.santos/app/node_modules/eslint-module-utils/moduleVisitor.js:67:5)
    at visitors.CallExpression (/Users/cesar.santos/app/node_modules/eslint-module-utils/moduleVisitor.js:105:29)
    at ruleErrorHandler (/Users/cesar.santos/app/node_modules/eslint/lib/linter/linter.js:1115:28)
    ...

I'm not sure why this "node: null" is needed 👇

msw/package.json

Lines 16 to 17 in a54138a

"./browser": {
"node": null,

but removing it fixes my issue above

@jcnix
Copy link

jcnix commented Oct 23, 2023

This is what resolved it for me from the examples repository. I don't pretend to know what this does, but it works.

https://github.com/mswjs/examples/blob/main/examples/with-jest/jest.config.ts#L20

@arturmiglio
Copy link

This is what resolved it for me from the examples repository. I don't pretend to know what this does, but it works.

https://github.com/mswjs/examples/blob/main/examples/with-jest/jest.config.ts#L20

For me, this works only partially. As it breaks other imports.

@PaulWild
Copy link

PaulWild commented Oct 26, 2023

We are unfortunately seeing the same issue when upgrading to v2. This is blocking us from moving to the new version. It is a Nextjs project and works without issue in v1

@okhomenko
Copy link

cc @kettanaito

@DercilioFontes
Copy link

I have the same issue using react, vite and vitest.

"react": "^18.2.0",
"react-dom": "^18.2.0",
"@testing-library/jest-dom": "^6.1.4",
"@testing-library/react": "^14.0.0",
"vite": "^4.5.0",
"vitest": "^0.34.6",

System:
NodeJS : v18.18.2
npm : 9.8.1
OS : macOS 13.5.2

@kettanaito
Copy link
Member

kettanaito commented Oct 27, 2023

Why does this error happen?

Because JSDOM forces the browser export condition. In other words, JSDOM says "if a third-party package exports a browser field, use that." That's the default and a rather dangerous default at that. Why? Because JSDOM still runs in Node.js. More to that, JSDOM cannot have 100% browser compatibility by design, so forcing the browser export condition will subject your tests to failures more than necessary when working with packages that ship different code for different environments, like MSW does.

The msw/node import must never happen in the browser code. Thus, MSW has the following export condition to ensure that on the bundler level:

msw/package.json

Lines 23 to 24 in 1d1fbca

"./node": {
"browser": null,

The same is true for the msw/browser import in Node.js. This is the right setup to achieve publishing to multiple environments at once. The problem is not the package but how your test environment is configured. Export conditions is a relatively new feature is Node.js, and some tools are either lacking behind, don't understand it entirely, or, like JSDOM, assume dangerous defaults that put the users in confusing situations.

How to solve this?

Opt out of the browser export condition by adding this to your jest.config.js:

// jest.config.js 
module.exports = {
  testEnvironmentOptions: {
    customExportConditions: [''],
  }
}

This will force JSDOM to use the node (or default) export condition, which is the correct behavior.

If you encounter other import-related issued after this change, they aren't related and have to be addressed separately. Adding this recommendation to the migration guide as well so everyone could follow.

@textbook
Copy link
Author

Confirmed, now 🟢 passes in both testEnvironments - thanks!

@DercilioFontes
Copy link

I have the same issue using react, vite and vitest.

"react": "^18.2.0",
"react-dom": "^18.2.0",
"@testing-library/jest-dom": "^6.1.4",
"@testing-library/react": "^14.0.0",
"vite": "^4.5.0",
"vitest": "^0.34.6",

System: NodeJS : v18.18.2 npm : 9.8.1 OS : macOS 13.5.2

I see that it is working for vitest with this change in the vite config:

{
// ...
 define: {
      'process.env.NODE_ENV': `"${mode}"`,
    },
    build: {
      target: ['ES2022'],
      outDir: 'build',
    },
    server: {
      open: mode === 'development',  // <-- here (it was true before). It will be 'test' and will not open the server
      port: 8101,
    },
    // https://vitest.dev/config/
    test: {
      globals: true,
      environment: 'jsdom',
      setupFiles: 'src/setupTests.ts',
    },
// ...
}

@joshkel
Copy link
Contributor

joshkel commented Oct 30, 2023

Disabling Jest's customExportConditions everywhere seemed somewhat drastic to me; I was hoping to change just the interaction between Jest and MSW, instead of changing everything within Jest to make MSW happy. (And, in a brief test, I ran into problems with other packages not working correctly when I cleared customExportConditions.)

Instead, I'm using a custom Jest resolver to modify MSW's imports.

Please note: This is a hack. It's deliberately doing something not intended by the MSW project. Subsequent MSW updates may break it. If you do this and it causes problems, please do not report any resulting issues to this project.

To do this: In the Jest config, define your resolver:

  resolver: `${__dirname}/config/jest/resolver.js`,

And create it with the following contents:

// Based on https://github.com/microsoft/accessibility-insights-web/pull/5421#issuecomment-1109168149
module.exports = (path, options) => {
  // Call the defaultResolver, so we leverage its cache, error handling, etc.
  return options.defaultResolver(path, {
    ...options,
    // Use packageFilter to process parsed `package.json` before the resolution
    // (see https://www.npmjs.com/package/resolve#resolveid-opts-cb)
    packageFilter: (pkg) => {
      if (pkg.name === 'msw') {
        delete pkg.exports['./node'].browser;
      }
      if (pkg.name === '@mswjs/interceptors') {
        delete pkg.exports;
      }

      return pkg;
    },
  });
};

@joshkel
Copy link
Contributor

joshkel commented Oct 31, 2023

Here's an improved version of my custom Jest resolver that replaces customExportConditions just for MSW, while leaving Jest's browser export for other code that expects it. This allows tests of code that's intended to run in the browser to continue to use the browser versions of other packages, while following MSW's expectation and recommendation of using MSW's Node code within tests.

module.exports = (path, options) => {
  // Jest + jsdom acts like a browser (i.e., it looks for "browser" imports
  // under pkg.exports), but msw knows that you're operating in a Node
  // environment:
  //
  // https://github.com/mswjs/msw/issues/1786#issuecomment-1782559851
  //
  // The MSW project's recommended workaround is to disable Jest's
  // customExportConditions completely, so no packages use their browser's
  // versions.  We'll instead clear export conditions only for MSW.
  if (/^(msw|@mswjs\/interceptors)(\/|$)/.test(path)) {
    return options.defaultResolver(path, { ...options, conditions: [] });
  }

  return options.defaultResolver(path, options);
};

If this works for others, it may be worth adding as an option in the docs.

@kettanaito
Copy link
Member

kettanaito commented Nov 1, 2023

I was hoping to change just the interaction between Jest and MSW, instead of changing everything within Jest to make MSW happy.

JSDOM is broken export conditions-wise, I believe I explained it at length. It's not about making MSW happy, it's about resolving third-party modules using the node export while running in Node.js. This is how module resolution is supposed to work, and by using old tools you are not getting the modern JavaScript. If you don't believe me, try Vitest. It works with the correct module resolution out of the box.

@joshkel, your custom resolver implementation looks interesting. I will leave it up to the people to decide which approach works best for them, but may I please stress this one last and final time: You need all this because you are using an OLD testing framework. Whether you decide to ignore the browser export condition in Node, which is what you should be doing, to begin with, or introduce a custom resolver for Jest—nothing of this has anything to do with MSW. Hope we get this straight in the discussions to come. Thanks.

@cleysonsilvame
Copy link

Why does this error happen?

Because JSDOM forces the browser export condition. In other words, JSDOM says "if a third-party package exports a browser field, use that." That's the default and a rather dangerous default at that. Why? Because JSDOM still runs in Node.js. More to that, JSDOM cannot have 100% browser compatibility by design, so forcing the browser export condition will subject your tests to failures more than necessary when working with packages that ship different code for different environments, like MSW does.

The msw/node import must never happen in the browser code. Thus, MSW has the following export condition to ensure that on the bundler level:

msw/package.json

Lines 23 to 24 in 1d1fbca

"./node": {
"browser": null,

The same is true for the msw/browser import in Node.js. This is the right setup to achieve publishing to multiple environments at once. The problem is not the package but how your test environment is configured. Export conditions is a relatively new feature is Node.js, and some tools are either lacking behind, don't understand it entirely, or, like JSDOM, assume dangerous defaults that put the users in confusing situations.

How to solve this?

Opt out of the browser export condition by adding this to your jest.config.js:

// jest.config.js 
module.exports = {
  testEnvironmentOptions: {
    customExportConditions: [''],
  }
}

This will force JSDOM to use the node (or default) export condition, which is the correct behavior.

If you encounter other import-related issued after this change, they aren't related and have to be addressed separately. Adding this recommendation to the migration guide as well so everyone could follow.

I have a similar situation

Inside the test environment (using vitest), I get the same error, because with ./browser.

If I follow the instruction to remove the conditional node, or put an empty string in the array, I end up breaking other tests that need the conditional node

Does anyone have any suggestions?

@DercilioFontes
Copy link

Why does this error happen?

Because JSDOM forces the browser export condition. In other words, JSDOM says "if a third-party package exports a browser field, use that." That's the default and a rather dangerous default at that. Why? Because JSDOM still runs in Node.js. More to that, JSDOM cannot have 100% browser compatibility by design, so forcing the browser export condition will subject your tests to failures more than necessary when working with packages that ship different code for different environments, like MSW does.
The msw/node import must never happen in the browser code. Thus, MSW has the following export condition to ensure that on the bundler level:

msw/package.json

Lines 23 to 24 in 1d1fbca

"./node": {
"browser": null,

The same is true for the msw/browser import in Node.js. This is the right setup to achieve publishing to multiple environments at once. The problem is not the package but how your test environment is configured. Export conditions is a relatively new feature is Node.js, and some tools are either lacking behind, don't understand it entirely, or, like JSDOM, assume dangerous defaults that put the users in confusing situations.

How to solve this?

Opt out of the browser export condition by adding this to your jest.config.js:

// jest.config.js 
module.exports = {
  testEnvironmentOptions: {
    customExportConditions: [''],
  }
}

This will force JSDOM to use the node (or default) export condition, which is the correct behavior.
If you encounter other import-related issued after this change, they aren't related and have to be addressed separately. Adding this recommendation to the migration guide as well so everyone could follow.

I have a similar situation

Inside the test environment (using vitest), I get the same error, because with ./browser.

If I follow the instruction to remove the conditional node, or put an empty string in the array, I end up breaking other tests that need the conditional node

Does anyone have any suggestions?

I put a vitest config example here (#1786 (comment)). It is working for me. No need to do anything else.

@kettanaito
Copy link
Member

kettanaito commented Nov 2, 2023

Please refer to the usage examples with Vitest that feature both ESM and CJS tests, all functional with MSW v2.0:

I've also just migrated the entire internal test suite of MSW to Vitest yesterday without any issues. You don't need to configure JSDOM in any way for Vitest+MSW to work.

@joshkel
Copy link
Contributor

joshkel commented Nov 2, 2023

Thanks for the reply, @kettanaito (and for your patience).

If I'm understanding correctly, it's not a matter of Jest being old or not supporting modern module resolution. (Jest fully supports modern modules and module resolution, as far as I know - somewhat held back by issues in Node.js itself.) Instead, it's a legitimate trade-off:

  • There are advantages to emulating the browser as closely as possible when testing browser code. (It's the same testing philosophy as Kent Dodds, I guess.)
  • There are advantages to being honest and accepting that the code runs in Node.js. The resulting code can be much simpler and can leverage pre-existing functionality (although that functionality may not behave the same as it does in the browser).

Jest and jsdom seem to default to the first approach. jsdom won't use implementations if they're not W3C-spec-compliant, and Jest uses the VM API (I think) to exclude Node.js globals (so code under test won't mistakenly reference them), and Jest decides to emulate the browser as closely as possible when using jsdom, including honoring the "browser" export conditions of Node modules. (And, since Jest's jsdom environment doesn't expose Node.js globals, some packages need to have their "browser" export conditions honored, so they'll reference browser globals rather than Node.)

Vitest takes the second approach (e.g., it exposes Buffer because "TODO a lot of dependencies use it") - but that means I can have production browser-side code that mistakenly references Buffer and passes tests and fails at runtime. MSW 2 picks the second approach (for reasons that you explain in depth), and I'm certain those are the correct tradeoffs for MSW - but now I have test failures because new FormData(form) no longer works. (This is not a complaint and not your problem.)

It wouldn't surprise me if Jest's tradeoffs are worse in general. (They've certainly caused a lot of complexity.) But it doesn't seem quite fair to say that picking a worse set of tradeoffs is broken. (I'm interested in switching to Vitest, but parts of the ecosystem still aren't there for my project.)

I'm happy to accept MSW's decision as to appropriate tradeoffs when using it. (That's what I meant by "making MSW happy"; I apologize if I contributed to any confusion.)

@mkalvas
Copy link

mkalvas commented Nov 2, 2023

may I please stress this one last and final time: You need all this because you are using an OLD testing framework. Whether you decide to ignore the browser export condition in Node, which is what you should be doing, to begin with, or introduce a custom resolver for Jest—nothing of this has anything to do with MSW. Hope we get this straight in the discussions to come.

Sorry to drag this out more, but this is just not accurate @kettanaito. I understand the technical details of your explanation and the differences between jest, jsdom, vitest, etc. perfectly, so don't mistake this for lack of understanding. There's no need for reiteration of your position, rather there's a need for listening and understanding your users' (valid) choices and circumstances.

Jest and jsdom are not "OLD" or outdated. As @joshkel mentioned, they provide a far more accurate testing environment for browser code. They are actively maintained and updated. They support the bleeding edge of browser changes and many wide-sweeping libraries and tools.

I've already spent far too much time trying to work around this issue including your suggestion, using other DOM implementations that fall short (such as happy-dom), polyfills, resolvers, etc. None of these are fully acceptable and have side effects that impact other correct code outside of msw. Please note that just because I was able to get my tests to run and pass, that does not mean that the overall outcome is acceptable.

As such, this is a total blocker for us. We will not be sacrificing the accuracy of our tests in relation to the production environment they'll be running in for one library. Somehow all the thousands of other libraries we use haven't had this issue.

This could be fixed for your users with some action on your part and you're choosing not to. That said, I understand if this is the direction you're taking the library. I'm not asking for anything and I expect nothing in return. I simply wanted to share feedback with you about the entrenched position you've taken that is very clearly not cut and dry like you've painted it.

You spend your time making this awesome tool and I appreciate that, it's just that I'll be unable to continue using it going forward. Thanks for all of your hard work and maybe I'll still be able to use it on some other non-browser project because I really do like it a lot ❤️

@jd-carroll

This comment was marked as outdated.

@jd-carroll
Copy link

Nope, my hopes were dashed. The custom resolver solution above is working great for me too.

@hhsl
Copy link

hhsl commented Jan 15, 2024

Please refer to the usage examples with Vitest that feature both ESM and CJS tests, all functional with MSW v2.0:

* [Vitest](https://github.com/mswjs/examples/tree/main/examples/with-vitest)

* [Vitest CJS](https://github.com/mswjs/examples/tree/main/examples/with-vitest-cjs)

I've also just migrated the entire internal test suite of MSW to Vitest yesterday without any issues. You don't need to configure JSDOM in any way for Vitest+MSW to work.

Just because I struggled a bit, I wanted to share our solution to this Error because this didn't help in our case while using Vitest. We were trying all attempts described here whithout succes. But then discoverd that the Error was thrown during coverage report generation. Our reporter "@vitest/coverage-istanbul" apparently looks at everything INSIDE our src/ folder, where we also saved our server mocks. So after moving our server mocks OUTSIDE of src/ the tests ran fine again.

I also have this little sandbox to reproduce this.

@Fremen1990
Copy link

I have the same issue using react, vite and vitest.

"react": "^18.2.0",
"react-dom": "^18.2.0",
"@testing-library/jest-dom": "^6.1.4",
"@testing-library/react": "^14.0.0",
"vite": "^4.5.0",
"vitest": "^0.34.6",

System: NodeJS : v18.18.2 npm : 9.8.1 OS : macOS 13.5.2

React Vite Vitest & MSW v2 still doesnt work 😵

I have same problem on React 18, Vite 5.1.0 , Vitest 1.2.2 and MSW 2.0

See no resolution for that on Vite and Vitest config.

I have checked example in Vitest from @kettanaito, but I cannot see the solution there...

Like @mkalvas said, there should be separated solution in documentation below Jest solution, as Vitest is huge communitt as well.

https://mswjs.io/docs/migrations/1.x-to-2.x/#cannot-find-module-mswnode-jsdom

Waiting for solution guide for Vitest 🧪

jg210 added a commit to jg210/spring-experiments that referenced this issue Feb 8, 2024
So far have had to stop jsdom declaring that tests are running in a browser:

mswjs/msw#1786

Fiddle with whatwg-fetch polyfill:

reduxjs/redux-toolkit#1271

Still more things to fix, some presumably a consequence of first issue.
@andrewbeckman
Copy link

Here's an improved version of my custom Jest resolver that replaces customExportConditions just for MSW, while leaving Jest's browser export for other code that expects it. This allows tests of code that's intended to run in the browser to continue to use the browser versions of other packages, while following MSW's expectation and recommendation of using MSW's Node code within tests.

module.exports = (path, options) => {
  // Jest + jsdom acts like a browser (i.e., it looks for "browser" imports
  // under pkg.exports), but msw knows that you're operating in a Node
  // environment:
  //
  // https://github.com/mswjs/msw/issues/1786#issuecomment-1782559851
  //
  // The MSW project's recommended workaround is to disable Jest's
  // customExportConditions completely, so no packages use their browser's
  // versions.  We'll instead clear export conditions only for MSW.
  if (/^(msw|@mswjs\/interceptors)(\/|$)/.test(path)) {
    return options.defaultResolver(path, { ...options, conditions: [] });
  }

  return options.defaultResolver(path, options);
};

If this works for others, it may be worth adding as an option in the docs.

@joshkel thanks so much for this! Saved me a ton of time. I made one small tweak to this:

if (/^(msw|@mswjs\/interceptors)(\/|$)/.test(path)) {
    return options.defaultResolver(path, {
      ...options,
      conditions: options.conditions.filter(
        (condition) => condition !== "browser"
      ),
    })
}

Namely, I left the conditions intact aside from filtering out browser. I'm still early in my testing, but this seems to be working for me and preventing another issue I was running into when I overwrote conditions to be empty.

pachun added a commit to pachun/too-many-men that referenced this issue Feb 22, 2024
As a beer league hockey player,
Given it's light outside,
When I open the Wolfpack app,
Then I see a light-themed app,
So that it's easier on my eyes.

As a beer league hockey player,
Given it's dark outside,
When I open the Wolfpack app,
Then I see a dark-themed app,
So that it's easier on my eyes.

https://www.pivotaltracker.com/story/show/186950237

Some other notes:

iOS uses [PlatformColor](https://reactnative.dev/docs/platformcolor)'s,
while other platforms use regular [react native color keywords](https://reactnative.dev/docs/colors#color-keywords).

Because of this, in order to test all the logic for a single color
scheme (we use light), we needed to test the behavior on another
platform (Android and web).

The default platform that we'd been testing was iOS - only iOS.

We now test on iOS and Android.

We would have liked to have tested all of iOS, Android, and web, since
we deploy all three, but we had [trouble getting MSW to work in the web
environment](mswjs/msw#1786).

We tried [the official solution in the docs](https://mswjs.io/docs/migrations/1.x-to-2.x#cannot-find-module-mswnode-jsdom)
and several solutions mentioned in the issue page, linked above. None of
those solutions worked well for us.

In lieu of testing the web env, we're going to try to only do iOS
specific things, and then any other logic that's used should be kept to
work for both Android and the web environment, so that all the web
environment features are still under test in the android test suite. The
solution isn't ideal, but as long as we stick with this approach (and
it'd be easy to forget or not do things this way, which is why it isn't
ideal) all the specs should cover all scenarios in every environment.

It's also worth noting that we decided not to test other color schemes
(we only test light by default) because testing the dark color scheme
seems like it won't add any value, unless we do [snapshot
tests](https://jestjs.io/docs/snapshot-testing), which don't align
well with the spirit of TDD (you write them after the fact) and would
promote difficulting in refactoring (changing the <View> structure of
a page (similar to changing the <div> structure of a webpage)) would
cause a test to fail - which isn't what we're looking for in tests. We
like to know that our feature specs still work after a change is made
- which is to say we like to have confidence that our refactors don't
change behavior, not that they don't change exactly _how_ features are
implemented.
pachun added a commit to pachun/too-many-men that referenced this issue Feb 22, 2024
As a beer league hockey player,
Given it's light outside,
When I open the Wolfpack app,
Then I see a light-themed app,
So that it's easier on my eyes.

As a beer league hockey player,
Given it's dark outside,
When I open the Wolfpack app,
Then I see a dark-themed app,
So that it's easier on my eyes.

https://www.pivotaltracker.com/story/show/186950237

Some other notes:

iOS uses [PlatformColor](https://reactnative.dev/docs/platformcolor)'s,
while other platforms use regular [react native color keywords](https://reactnative.dev/docs/colors#color-keywords).

Because of this, in order to test all the logic for a single color
scheme (we use light), we needed to test the behavior on another
platform (Android and web).

The default platform that we'd been testing was iOS - only iOS.

We now test on iOS and Android.

We would have liked to have tested all of iOS, Android, and web, since
we deploy all three, but we had [trouble getting MSW to work in the web
environment](mswjs/msw#1786).

We tried [the official solution in the docs](https://mswjs.io/docs/migrations/1.x-to-2.x#cannot-find-module-mswnode-jsdom)
and several solutions mentioned in the issue page, linked above. None of
those solutions worked well for us.

In lieu of testing the web env, we're going to try to only do iOS
specific things, and then any other logic that's used should be kept to
work for both Android and the web environment, so that all the web
environment features are still under test in the android test suite. The
solution isn't ideal, but as long as we stick with this approach (and
it'd be easy to forget or not do things this way, which is why it isn't
ideal) all the specs should cover all scenarios in every environment.

It's also worth noting that we decided not to test other color schemes
(we only test light by default) because testing the dark color scheme
seems like it won't add any value, unless we do [snapshot
tests](https://jestjs.io/docs/snapshot-testing), which don't align
well with the spirit of TDD (you write them after the fact) and would
promote difficulting in refactoring (changing the <View> structure of
a page (similar to changing the <div> structure of a webpage)) would
cause a test to fail - which isn't what we're looking for in tests. We
like to know that our feature specs still work after a change is made
- which is to say we like to have confidence that our refactors don't
change behavior, not that they don't change exactly _how_ features are
implemented.
@essana3
Copy link

essana3 commented Apr 7, 2024

Still having the issue with vite, vitest and react...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node
Projects
None yet
Development

No branches or pull requests