chore: updated msw and migrated from rest to http and HttpResponse

[nilsingwersen]

What does it do?

I updated msw to 2 the newest version and migrated the tests.

Why is it needed?

In the msw 1 version there is a peer dependency to an older typescript version which throws a warning when installing strapi.

How to test it?

I tested it by running all frontend tests.

Related issue(s)/PR(s)

refers to #21201

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
contributor-docs	❌ Failed (Inspect)			Oct 21, 2024 0:15am

[strapi-cla]

All committers have signed the CLA.

[innerdvations]

A bunch of front-end tests are failing with "ReferenceError: TextEncoder is not defined", that seem related to this. Can you see what the issue could be?

[nilsingwersen]

A bunch of front-end tests are failing with "ReferenceError: TextEncoder is not defined", that seem related to this. Can you see what the issue could be?

This is probably related to this:

https://github.com/mswjs/msw/issues/1796

So I guess the jest setup needs to be modified.

When running the tests locally I ran yarn test:ts:front but now when running yarn test:front I get the same error.

The fix is commented in the docs here:

https://mswjs.io/docs/migrations/1.x-to-2.x#requestresponsetextencoder-is-not-defined-jest

[nilsingwersen]

I tested it now with jest-fixed-jsdom. The custom environment.ts seems to come from the same problem with the this.global.structuredClone = structuredClone;

There were still some tests failing but there you probably know more than me.

Alternatively to the extra dependency in the custom environment this could be added:

this.global.TextDecoder = TextDecoder; this.global.TextEncoder = TextEncoder; this.global.ReadableStream = ReadableStream;

this.global.Blob = Blob; this.global.Headers = Headers; this.global.FormData = FormData; this.global.Request = Request; this.global.Response = Response; this.global.fetch = fetch; this.global.structuredClone = structuredClone; this.global.URL = URL;

This is what the package currently does

[jhoward1994]

I would rather we still manage our own test environment and add to this.global than rely on jest-fixed-jsdom wdyt @Bassel17 @butcherZ ?

[nilsingwersen]

I would rather we still manage our own test environment and add to this.global than rely on jest-fixed-jsdom wdyt @Bassel17 @butcherZ ?

I reverted to using the CustomJSDOMEnvironment and added to it what is in jest-fixed-jsdom.

Can I help with something else?

[jhoward1994]

Thank you! I'm still seeing some errors e.g. in packages/core/content-manager/admin/src/pages/tests/EditConfigurationPage.test.tsx (and similar cases elsewhere)

...
    > 85 |     await findByText('Saved');
         |           ^
      86 |   });
      87 |
      88 |   it('should delete field', async () => {

      at waitForWrapper (../../../node_modules/@testing-library/dom/dist/wait-for.js:163:27)
      at ../../../node_modules/@testing-library/dom/dist/query-helpers.js:86:33
      at Object.findByText (admin/src/pages/tests/EditConfigurationPage.test.tsx:85:11)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 skipped, 1 passed, 3 total
Snapshots:   0 total
Time:        8.465 s, estimated 10 s
Ran all test suites matching /packages\/core\/content-manager\/admin\/src\/pages\/tests\/EditConfigurationPage.test.tsx/i.

Do you see the same?

I think this must be something to do with the msw changes in packages/core/content-manager/admin/tests/server.ts

[nilsingwersen]

Thank you! I'm still seeing some errors e.g. in packages/core/content-manager/admin/src/pages/tests/EditConfigurationPage.test.tsx (and similar cases elsewhere)

...
    > 85 |     await findByText('Saved');
         |           ^
      86 |   });
      87 |
      88 |   it('should delete field', async () => {

      at waitForWrapper (../../../node_modules/@testing-library/dom/dist/wait-for.js:163:27)
      at ../../../node_modules/@testing-library/dom/dist/query-helpers.js:86:33
      at Object.findByText (admin/src/pages/tests/EditConfigurationPage.test.tsx:85:11)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 skipped, 1 passed, 3 total
Snapshots:   0 total
Time:        8.465 s, estimated 10 s
Ran all test suites matching /packages\/core\/content-manager\/admin\/src\/pages\/tests\/EditConfigurationPage.test.tsx/i.

Do you see the same?

I think this must be something to do with the msw changes in packages/core/content-manager/admin/tests/server.ts

Yeah I see the same but I'm not sure if this isn't actually right. Could you verify that these test results are wrong?

I kinda tried debbuging this with claude and showed it a test an I got this answer:

The problem is that renderHook doesn't render anything to the DOM - it only tests the hook's behavior. That's why screen.findByText can't find the text - there's no actual DOM being rendered!

this was for useDocumentActions.test.ts

I don't know how to verify this quickly

[jhoward1994]

Hi, sorry for the slow response. It seems to me that these test failures are genuine frontend errors and are caused by incorrectly mocked MSW response since the upgrade

e.g. for the failures in packages/core/content-manager/admin/src/pages/tests/EditConfigurationPage.test.tsx

I was able to resolve these by making this change to packages/core/content-manager/admin/tests/server.ts L15

  http.put('/content-manager/content-types/:model/configuration', ({ params }) => {
    return HttpResponse.json(
      {
        data:
          params.model === 'api::homepage.homepage'
            ? mockData.contentManager.singleTypeConfiguration
            : mockData.contentManager.collectionTypeConfiguration,
      },
      { status: 200 }
    );
  }),

e.g. your change was to return new HttpResponse(null, { status: 200 }); when in fact that part of the application relies on JSON returned in { data: ... }

I think this will apply for the other failures too, feel free to ping me when they're resolved or if you need a hand 🙏🏻

[hanpaine]

Hi @nilsingwersen is this still on your radar? 🙏

[nilsingwersen]

Yes, I'm sorry, I was a little unmotivated because it seems like msw 2 is a little stricter, which is a good thing but also means that a few more tests need to be adjusted. I'll get back to on the upcoming weekend.

[hanpaine]

No need to apologise - I appreciate the effort and time 🙏

[jhoward1994]

Closing for https://github.com/strapi/strapi/pull/22615