Skip to content

Conversation

erwanMarmelab
Copy link
Contributor

@erwanMarmelab erwanMarmelab commented Jul 21, 2025

Problem

fetch-mock brings us back some vulnerabilities.

Solution

Change it for MSW

To Do

  • demo
  • crm
  • simple

Additional Checks

  • The PR targets master for a bugfix or a documentation fix, or next for a feature

const data = generateData();
const restServer = JsonGraphqlServer({ data });
const handler = restServer.getHandler();
const handlerWithLogs = (url: string, opts: any) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to keep it. If we don't pass quiet: true to worker.start, it will automatically displays us the right logs 👍

@erwanMarmelab erwanMarmelab added RFR Ready For Review and removed WIP Work In Progress labels Jul 21, 2025
@djhi djhi self-requested a review July 21, 2025 15:24
Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Good work, except you probably didn't test the graphql demo which is broken

},
"msw": {
"workerDirectory": [
"assets",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why assets? mockServiceWorker.js is in public directory, not assets.

@fzaninotto
Copy link
Member

needs rebase

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

The e-commerce demo doesn't work (either with REST or GraphQL). I get this error in the SAP when running make run-demo:

error loading dynamically imported module: http://localhost:8000/src/dashboard/OrderChart.tsx

});
export const worker = setupWorker(http.all(/http:\/\/localhost:4000/, handler));

export default () => worker;
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this export? I don't see it imported anywhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -7361,6 +7496,13 @@ __metadata:
languageName: node
linkType: hard

"cli-width@npm:^4.1.0":
Copy link
Member

Choose a reason for hiding this comment

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

This one is duplicated (albeit in a different major version). Can we avoid it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't deduplicate different major versions

@@ -7734,6 +7876,13 @@ __metadata:
languageName: node
linkType: hard

"cookie@npm:^0.7.2":
Copy link
Member

Choose a reason for hiding this comment

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

also duplicated

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same with an additional note: before 1.0.0, all versions can have breaking changes

yarn.lock Outdated
@@ -9862,6 +10003,15 @@ __metadata:
languageName: node
linkType: hard

"fakerest@npm:4.1.3, fakerest@npm:^4.1.3":
Copy link
Member

Choose a reason for hiding this comment

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

also duplicated

yarn.lock Outdated
version: 4.3.2
resolution: "linkifyjs@npm:4.3.2"
checksum: 1a85e6b368304a4417567fe5e38651681e3e82465590836942d1b4f3c834cc35532898eb1e2479f6337d9144b297d418eb708b6be8ed0b3dc3954a3588e07971
version: 4.1.1
Copy link
Member

Choose a reason for hiding this comment

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

please revert this one, linkify < 4.3.2 has a security vulnerability

yarn.lock Outdated
@@ -18802,6 +18992,18 @@ __metadata:
languageName: node
linkType: hard

"tough-cookie@npm:^4.1.4":
Copy link
Member

Choose a reason for hiding this comment

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

duplicate package

@@ -14,6 +14,7 @@
"@mui/material": "^5.16.12",
"@tanstack/react-query": "^5.83.0",
"@tanstack/react-query-devtools": "^5.83.0",
"fakerest": "^4.1.3",
"jsonexport": "^3.2.0",
"lodash": "~4.17.5",
"ra-data-fakerest": "^5.10.0",
Copy link
Member

Choose a reason for hiding this comment

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

no longer needed. Also, missing ra-data-simple-rest.

@djhi
Copy link
Collaborator

djhi commented Jul 31, 2025

The e-commerce demo doesn't work (either with REST or GraphQL). I get this error in the SAP when running make run-demo:

I can't reproduce this. Can you try resetting your node_modules?

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

The error was caused by Firefox being Firefox. Rebooting it fixed it.

@djhi djhi merged commit 42d7465 into master Jul 31, 2025
33 of 35 checks passed
@djhi djhi deleted the demo/msw branch July 31, 2025 13:45
@djhi djhi added this to the 5.10.1 milestone Aug 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Development

Successfully merging this pull request may close these issues.

4 participants