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

Upgrade query-string to v9 #9812

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

MohammedFaragallah
Copy link
Contributor

No description provided.

@@ -28,7 +28,7 @@ module.exports = {
'/packages/create-react-admin/templates',
],
transformIgnorePatterns: [
'[/\\\\]node_modules[/\\\\](?!(@hookform)/).+\\.(js|jsx|mjs|ts|tsx)$',
'[/\\\\]node_modules[/\\\\](?!(@hookform|query-string|decode-uri-component|filter-obj|split-on-first)/).+\\.(js|jsx|mjs|ts|tsx)$',
Copy link
Member

Choose a reason for hiding this comment

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

does that mean that react-admin users will have to do the same in their jest configuration?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, this is specific to Jest

Copy link
Member

Choose a reason for hiding this comment

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

This is exactly my question

Copy link
Contributor

@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.

We currently have three libraries in our yarn.lock for query string parsing/formatting:

  • qs (used by storybook)
  • query-string
  • querystring (used by fetch-mock)

Maybe we could use one already there from our deps?

@djhi djhi closed this May 6, 2024
@djhi djhi reopened this May 6, 2024
@djhi
Copy link
Contributor

djhi commented May 6, 2024

Sorry for the noise. query-string actually does more that URLSearchParams and we decided to keep it. Can you rebase or merge the latest next branch? Thanks!

@MohammedFaragallah
Copy link
Contributor Author

Sorry for the noise. query-string actually does more that URLSearchParams and we decided to keep it. Can you rebase or merge the latest next branch? Thanks!

No worries happy to help any time, Done.

@@ -687,7 +687,7 @@ Here is an example implementation, that you can use as a base for your own Data

```js
import { fetchUtils } from 'react-admin';
import { stringify } from 'query-string';
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to do that? I see no upgrade guide in query-string for that change.

Choose a reason for hiding this comment

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

yeah, this package just export default now

@fzaninotto
Copy link
Member

Thanks for your comment. Can you rebase to fix the merge conflicts?

@MohammedFaragallah
Copy link
Contributor Author

Thanks for your comment. Can you rebase to fix the merge conflicts?

Done but one of the checks is failing and there's no log

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

Successfully merging this pull request may close these issues.

None yet

4 participants