chore(clerk-js): Replace qs library with custom implementation by EmmanouelaPothitou · Pull Request #3430 · clerk/javascript

EmmanouelaPothitou · 2024-05-23T10:34:37Z

Description

Removing the qs library and using the native URLSearchParams API instead reduced the bundle size of clerk.browser.js by 13.5% (from 70.14KB to 60.62KB) and clerk.headless.js by 22% (from 46.5KB to 36.5KB).

Checklist

npm test runs as expected.
npm run build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
(If applicable) Documentation has been updated

Type of change

changeset-bot · 2024-05-23T10:34:41Z

🦋 Changeset detected

Latest commit: 371c22d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@clerk/clerk-js	Patch
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

nikosdouvlis · 2024-05-23T13:14:57Z

Good job so far. Please go through the PR comments and once you're ready, don't forget to:

Update the PRs description to explain why we're doing this change. Also, can you calculate the change in bundle size?
Add a few unit tests that cover fapiClient and the encoders used

nikosdouvlis · 2024-06-05T18:23:18Z

@EmmanouelaPothitou This looks good to me. Can you include the size reduction in the description?

Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com>

…m function

Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com>

dimkl

🤔 In case the stringifyQueryParams is being used in the request url, i think we should also urlEncode the values to avoid cases of strings containing invalid url symbols (eg =).
Could we test how the qs behaves in those cases?

EmmanouelaPothitou · 2024-06-10T12:43:26Z

🤔 In case the stringifyQueryParams is being used in the request url, i think we should also urlEncode the values to avoid cases of strings containing invalid url symbols (eg =). Could we test how the qs behaves in those cases?

Using the append method on a URLSearchParams instance automatically encodes the value, so I think we don't need to use urlEncode.

https://url.spec.whatwg.org/#example-constructing-urlsearchparams

EmmanouelaPothitou · 2024-06-11T08:39:41Z

!snapshot

clerk-cookie · 2024-06-11T08:43:27Z

Hey @EmmanouelaPothitou - the snapshot version command generated the following package versions:

Package	Version
@clerk/chrome-extension	1.0.18-snapshot.v11a5fcc
@clerk/clerk-js	5.6.1-snapshot.v11a5fcc
@clerk/clerk-expo	1.2.1-snapshot.v11a5fcc
gatsby-plugin-clerk	5.0.0-beta.45

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/chrome-extension

npm i @clerk/chrome-extension@1.0.18-snapshot.v11a5fcc --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.6.1-snapshot.v11a5fcc --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@1.2.1-snapshot.v11a5fcc --save-exact

gatsby-plugin-clerk

npm i gatsby-plugin-clerk@5.0.0-beta.45 --save-exact

nikosdouvlis · 2024-06-11T09:53:11Z

Will be released once #3518 is out

…h-query-string

nikosdouvlis · 2024-06-12T12:32:48Z

Great job @EmmanouelaPothitou!

LekoArts · 2024-06-12T12:58:46Z

+  // The return value isn't as expected.
+  // The buildUrl function converts an undefined value to the string 'undefined'
+  // and includes it in the search parameters.
+  it.skip('parses search params when value is undefined', () => {


Can we unskip this?

The buildUrl function converts an undefined value to the string 'undefined' and includes it in the search parameters. This issue existed before removing the qs library, so I didn't address it in this PR.
The qs library used to exclude keys with undefined values from the search parameter string. The stringifyQueryParams function should behave like qs.

I will now create a task in Linear to investigate it further. Therefore, I think we should skip this test for now.

EmmanouelaPothitou requested a review from nikosdouvlis May 23, 2024 10:34

clerk-cookie added the clerk-js label May 23, 2024

nikosdouvlis reviewed May 23, 2024

View reviewed changes

EmmanouelaPothitou marked this pull request as ready for review June 5, 2024 10:01

nikosdouvlis reviewed Jun 5, 2024

View reviewed changes

Comment thread packages/clerk-js/src/core/fapiClient.ts Outdated

Comment thread packages/clerk-js/src/utils/__tests__/querystring.test.ts Outdated

Comment thread packages/clerk-js/src/utils/__tests__/querystring.test.ts

nikosdouvlis reviewed Jun 5, 2024

View reviewed changes

Comment thread packages/clerk-js/src/utils/querystring.ts Outdated

nikosdouvlis closed this Jun 5, 2024

nikosdouvlis force-pushed the emmanouela/sdk-873-attempt-to-replace-qs-with-query-string branch from e8c83ab to 4878289 Compare June 5, 2024 18:04

nikosdouvlis deleted the emmanouela/sdk-873-attempt-to-replace-qs-with-query-string branch June 5, 2024 18:07

nikosdouvlis reopened this Jun 5, 2024

nikosdouvlis force-pushed the emmanouela/sdk-873-attempt-to-replace-qs-with-query-string branch from f95319b to 773d959 Compare June 5, 2024 18:50

nikosdouvlis changed the title ~~chore(clerk-js): Remove qs library~~ chore(clerk-js): Replace qs library with custom implementation Jun 6, 2024

nikosdouvlis requested review from anagstef and dimkl June 6, 2024 10:00

EmmanouelaPothitou and others added 11 commits June 6, 2024 13:34

chore(clerk-js): Remove qs library

849499d

chore(clerk-js): Create stringifyQueryParams function

02e100e

Update packages/clerk-js/src/utils/querystring.ts

ac0477c

Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com>

chore(clerk-js): Update ParsedQueryString type and stringifyQueryPara…

9b08b09

…m function

chore(clerk-js): Update querystring.ts file and add more tests

d812bc9

Create silent-countries-jump.md

8d17948

Update packages/clerk-js/src/core/fapiClient.ts

8a070ff

Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com>

chore(clerk-js): Rename encoder to keyEncoder

f47f9ae

Resolve PR comments

bfc7429

Update package-lock.json

2fbf8db

chore(repo): Update bundlewatch config

cb77adf

nikosdouvlis force-pushed the emmanouela/sdk-873-attempt-to-replace-qs-with-query-string branch from f43bc02 to cb77adf Compare June 6, 2024 10:34

dimkl reviewed Jun 6, 2024

View reviewed changes

Comment thread packages/clerk-js/src/utils/querystring.ts Outdated

Comment thread packages/clerk-js/src/utils/querystring.ts Outdated

dimkl approved these changes Jun 6, 2024

View reviewed changes

EmmanouelaPothitou force-pushed the emmanouela/sdk-873-attempt-to-replace-qs-with-query-string branch from 2b83e76 to 34313aa Compare June 10, 2024 11:44

chore(clerk-js): Add more tests on querystring and fapiClient

11a5fcc

EmmanouelaPothitou force-pushed the emmanouela/sdk-873-attempt-to-replace-qs-with-query-string branch from 34313aa to 11a5fcc Compare June 10, 2024 12:50

clerk deleted a comment from EmmanouelaPothitou Jun 10, 2024

clerk deleted a comment from EmmanouelaPothitou Jun 11, 2024

Merge branch 'main' into emmanouela/sdk-873-attempt-to-replace-qs-wit…

371c22d

…h-query-string

nikosdouvlis approved these changes Jun 12, 2024

View reviewed changes

nikosdouvlis merged commit 352afca into main Jun 12, 2024

clerk-cookie mentioned this pull request Jun 12, 2024

ci(repo): Version packages #3558

Merged

LekoArts reviewed Jun 12, 2024

View reviewed changes

octoper mentioned this pull request Sep 25, 2024

fix(clerk-js): Fix space encoding for query params #4219

Closed

9 tasks

Conversation

EmmanouelaPothitou commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Type of change

Uh oh!

changeset-bot Bot commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikosdouvlis commented May 23, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikosdouvlis commented Jun 5, 2024

Uh oh!

dimkl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

EmmanouelaPothitou commented Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EmmanouelaPothitou commented Jun 11, 2024

Uh oh!

clerk-cookie commented Jun 11, 2024

Uh oh!

nikosdouvlis commented Jun 11, 2024

Uh oh!

nikosdouvlis commented Jun 12, 2024

Uh oh!

LekoArts Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

EmmanouelaPothitou Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

EmmanouelaPothitou commented May 23, 2024 •

edited

Loading

changeset-bot Bot commented May 23, 2024 •

edited

Loading

EmmanouelaPothitou commented Jun 10, 2024 •

edited

Loading

EmmanouelaPothitou Jun 12, 2024 •

edited

Loading