Add Mock Service Worker - msw #513

jmcpeak · 2024-02-13T15:27:25Z

https://mswjs.io/docs/integrations/browser

webpro

Thanks @jmcpeak! Thanks a lot for the PR. I'd love to add a plugin for MSW, but also wonder what would be the best default configuration for entry files (I've added some comments).

webpro · 2024-02-15T06:51:01Z

packages/knip/src/plugins/msw/index.ts

+  'mocks/browser.{js,ts}',
+  'mocks/handlers.{js,ts}"',
+  'mocks/index.{js,ts}"',
+  'mocks/server.{js,ts}"',


Are these the default locations?

Feels like files like those listed and src/mocks/node.js could be located anywhere without clear defaults, right? If most projects would follow a certain convention that could work too, but I'm not aware of any. Do you have any insights or links that could help here?

The default everyone does is in a root directory of /mocks see all the examples - https://github.com/mswjs/examples/tree/main/examples/with-vitest - this is vitest, but its true for all examples.
And here is another example of the /mocks dir - https://mswjs.io/docs/integrations/browser#conditionally-enable-mocking

I'm not 100% sure if I'm using the correct knip functions to check for these files. The tests pass and it works - just not sure.

Also, if we could peek inside their package.json we could get the correct directory for mockServiceWorker.{js,,ts}:

"msw": { "workerDirectory": "public" }

Thanks!

The findReferences function receives a second options arguments which contains options.manifest.

I'm not 100% sure if I'm using the correct knip functions to check for these files. The tests pass and it works - just not sure.

Many tests are more "end to end" and use the main() function, then you can assert things on the returned issues and counters objects. See e.g. https://github.com/webpro/knip/blob/main/packages/knip/test/plugins/ava2.test.ts

Seems like the msw.workerDirectory value is not taken into account yet?

The patterns here don't have src/ included while the fixture does have that.

More importantly, the example you share shows that vitest.setup.ts imports ./mocks/node.js which in turn imports ./handlers.js. This makes me think none of the mocks/* files need to be configured in Knip, since vitest.setup.ts is already covered by the Vitest plugin.

Maybe you had some other plugin missing that led to the mock/* files not being included?

const localConfig = basename(configFilePath) === 'package.json' ? manifest.msw : undefined; const workerDirectory = localConfig?.workerDirectory; if (workerDirectory) { ENTRY_FILE_PATTERNS.push(`${workerDirectory}/mockServiceWorker.{js,ts}`) }

This bit of code looks for the entry somewhere else other than the default of public

packages/knip/src/plugins/next/index.ts

packages/knip/fixtures/plugins/msw/msw.toml

webpro

Looks like it comes down to adding (only) ${package.json#msw.workerDirectory}/mockServiceWorker.js into the process :)

package-lock.json

packages/knip/fixtures/plugins/msw/package-lock.json

packages/knip/src/plugins/msw/index.ts

packages/knip/src/plugins/next/index.ts

webpro · 2024-02-17T12:47:54Z

packages/knip/src/plugins/msw/index.ts

+  'mocks/browser.{js,ts}',
+  'mocks/handlers.{js,ts}"',
+  'mocks/index.{js,ts}"',
+  'mocks/server.{js,ts}"',


Seems like the msw.workerDirectory value is not taken into account yet?

The patterns here don't have src/ included while the fixture does have that.

More importantly, the example you share shows that vitest.setup.ts imports ./mocks/node.js which in turn imports ./handlers.js. This makes me think none of the mocks/* files need to be configured in Knip, since vitest.setup.ts is already covered by the Vitest plugin.

Maybe you had some other plugin missing that led to the mock/* files not being included?

webpro · 2024-02-17T12:54:55Z

packages/knip/test/plugins/msw.test.ts

+      join(cwd, 'src/mocks/handlers.ts'),
+      join(cwd, 'src/mocks/index.ts'),
+      join(cwd, 'src/mocks/server.ts'),
+    ])


The idea is that those files are not in issues.files. This test now asserts that the files are unused, which would defeat the purpose of the plugin.

Seems like the msw.workerDirectory value is not taken into account yet?

So I've been looking at this deeper - seems like it is really only used for the npx init script...

"Save the given PUBLIC_DIR in package.json for future automatic updates of the worker script."

https://mswjs.io/docs/cli/init/#flags

I can still put the check in, my guess is most people installing it don't use the --save flag.

Maybe you had some other plugin missing that led to the mock/* files not being included?

Nope, tried with vanilla knip install, these files are still listed as not used. I did not have vitest installed or configured in this vanilla install.

The patterns here don't have src/ included while the fixture does have that.

I didn't want to assume they create a /src directory - I used /src in my example test, guess I should have included them in my root / dir as well.

I can still put the check in, my guess is most people installing it don't use the --save flag.

This is the default, my guess is that MSW has a postinstall script that does this by default (also without --save).

Nope, tried with vanilla knip install, these files are still listed as not used.

I wonder what imports those files. Either there are source file(s) importing them, but somehow not part of the process, or they're imported by some external tool and we're missing a plugin. By now I'm inclined to think it's not MSW importing them automatically (except for the service worker).

Yeah, I don't know what to do about this one.

jmcpeak · 2024-02-23T14:31:08Z

Anything else you want me to modify? I have changed the code/tests to work properly now.

webpro · 2024-02-23T14:38:43Z

The conversations/questions are closed/marked "resolved" while in fact they aren't. In general I think it's better to have the reviewer mark them as resolved.

jmcpeak · 2024-02-23T14:52:52Z

The conversations/questions are closed/marked "resolved" while in fact they aren't. In general I think it's better to have the reviewer mark them as resolved.

My bad, I thought they we an indicator to you that I finished the work and you should take a look.

webpro · 2024-02-23T14:55:33Z

The thing is, I'm still not sure how to deal with those entry files. Iirc the example you've showed me imported mock/* file(s):

webpro · 2024-03-12T08:02:37Z

Let's wrap up this one. Here's how I think it should work:

Default entry is (only) ['mockServiceWorker.js']
In findDependencies we can look up localConfig?.workerDirectory and only if found, return the toEntryPattern(join()) of that dir and mockServiceWorker.js
No other entry patterns should be there, I think also mockServiceWorker.ts is not picked up by MSW?
I think that's it, really :)

Let me know you'd like to finish this (happy to have you be the contributor!), otherwise I'm happy to pick it up.

jmcpeak · 2024-03-12T15:56:30Z

Let's wrap up this one. Here's how I think it should work:

Default entry is (only) ['mockServiceWorker.js']

In findDependencies we can look up localConfig?.workerDirectory and only if found, return the toEntryPattern(join()) of that dir and mockServiceWorker.js

No other entry patterns should be there, I think also mockServiceWorker.ts is not picked up by MSW?

I think that's it, really :)

Let me know you'd like to finish this (happy to have you be the contributor!), otherwise I'm happy to pick it up.

I'd love to take it over the finish line - I will implement the changes you requested today

webpro · 2024-03-13T09:07:12Z

packages/docs/scripts/generate-plugin-docs.ts

@@ -10,7 +10,7 @@ import remarkStringify from 'remark-stringify';
 import { unified } from 'unified';
 import { u } from 'unist-builder';
 import { base } from '../config.js';
-import type { Plugin } from '../../knip/src/types/plugins.js';
+import type { Plugin } from 'knip/dist/types/plugins.ts';


I think this is an unrelated change?

webpro · 2024-03-13T09:08:17Z

packages/knip/src/plugins/msw/index.ts

@@ -1,3 +1,5 @@
+// eslint-disable-next-line n/no-restricted-import
+import path from 'node:path';


The restriction is there for a reason, please use the internal join.

webpro · 2024-03-13T09:09:31Z

packages/knip/test/plugins/msw.test.ts

  assert(issues.files.has(join(cwd, 'junk.js')));
  assert(issues.files.has(join(cwd, 'mocks/junk.js')));
  assert(issues.files.has(join(cwd, 'src/mocks/junk.js')));

  assert.deepEqual(counters, {
    ...baseCounters,
    devDependencies: 1,
-    files: 3,
+    files: 13,


Please remove unused files, we don't need to assert that functionality in this test.

webpro · 2024-03-13T09:11:57Z

package-lock.json

Please undo changes in the lockfile, I think they're unrelated?

They are, just forgot about it

don't know why tests are failing, they work locally

QA didn't pass because of this change: #513 (comment)

webpro · 2024-03-14T08:53:07Z

🚀 This pull request is included in v5.1.0. See Release 5.1.0 for release notes.

webpro · 2024-03-14T09:01:40Z

Thanks for bearing with me, Jason. Hope you don't mind I wrapped up the PR. It's out now! 🚀

jmcpeak · 2024-03-14T15:44:59Z

Thanks for bearing with me, Jason. Hope you don't mind I wrapped up the PR. It's out now! 🚀

Don't mind at all, glad that I could contribute!

webpro reviewed Feb 15, 2024

View reviewed changes

webpro reviewed Feb 17, 2024

View reviewed changes

webpro force-pushed the main branch from f5eb88c to 0074cc1 Compare March 8, 2024 10:07

webpro approved these changes Mar 13, 2024

View reviewed changes

webpro reviewed Mar 13, 2024

View reviewed changes

jmcpeak and others added 14 commits March 14, 2024 09:26

Add msw

d2211d4

PR feedback

7fc7773

Support Intellij project users

47d0d98

PR review

3a5a963

PR review

ad81767

PR review

0b98809

PR review

cc78f36

PR review

9d3ee32

PR review

723ef8d

PR review

646c907

PR review

73bda41

PR review

a1e6d50

PR review

213aa6e

Wrap up

bb39123

webpro force-pushed the jmcpeak/add-msw branch from b99b1c0 to bb39123 Compare March 14, 2024 08:32

Add msw to schema & types

a1a6bb3

webpro merged commit 1404348 into webpro-nl:main Mar 14, 2024
7 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Mock Service Worker - msw #513

Add Mock Service Worker - msw #513

jmcpeak commented Feb 13, 2024

webpro left a comment

webpro Feb 15, 2024

jmcpeak Feb 15, 2024

jmcpeak Feb 15, 2024

jmcpeak Feb 15, 2024 •

edited

Loading

webpro Feb 16, 2024

webpro Feb 16, 2024

webpro Feb 17, 2024

jmcpeak Feb 23, 2024

webpro left a comment

webpro Feb 17, 2024

webpro Feb 17, 2024

jmcpeak Feb 17, 2024 •

edited

Loading

jmcpeak Feb 17, 2024

jmcpeak Feb 17, 2024 •

edited

Loading

webpro Feb 17, 2024

jmcpeak Feb 23, 2024

jmcpeak commented Feb 23, 2024

webpro commented Feb 23, 2024

jmcpeak commented Feb 23, 2024

webpro commented Feb 23, 2024 •

edited

Loading

webpro commented Mar 12, 2024 •

edited

Loading

jmcpeak commented Mar 12, 2024 •

edited

Loading

webpro Mar 13, 2024

webpro Mar 13, 2024

webpro Mar 13, 2024

webpro Mar 13, 2024

jmcpeak Mar 13, 2024

jmcpeak Mar 13, 2024

webpro Mar 14, 2024

webpro commented Mar 14, 2024

webpro commented Mar 14, 2024

jmcpeak commented Mar 14, 2024

		@@ -1,3 +1,5 @@
		// eslint-disable-next-line n/no-restricted-import
		import path from 'node:path';

Add Mock Service Worker - msw #513

Add Mock Service Worker - msw #513

Conversation

jmcpeak commented Feb 13, 2024

webpro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmcpeak Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webpro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmcpeak Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmcpeak Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmcpeak commented Feb 23, 2024

webpro commented Feb 23, 2024

jmcpeak commented Feb 23, 2024

webpro commented Feb 23, 2024 • edited Loading

webpro commented Mar 12, 2024 • edited Loading

jmcpeak commented Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webpro commented Mar 14, 2024

webpro commented Mar 14, 2024

jmcpeak commented Mar 14, 2024

jmcpeak Feb 15, 2024 •

edited

Loading

jmcpeak Feb 17, 2024 •

edited

Loading

jmcpeak Feb 17, 2024 •

edited

Loading

webpro commented Feb 23, 2024 •

edited

Loading

webpro commented Mar 12, 2024 •

edited

Loading

jmcpeak commented Mar 12, 2024 •

edited

Loading