Skip to content

fix(userId): Prevent the id from resetting on every page refresh #182

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

Merged
merged 2 commits into from
Jul 13, 2023
Merged

fix(userId): Prevent the id from resetting on every page refresh #182

merged 2 commits into from
Jul 13, 2023

Conversation

duailibe
Copy link
Contributor

@duailibe duailibe commented Jul 13, 2023

The getUserId() function, when run for the first time, tries to fetch the user id from a cookie, but the current regex is not getting it correctly. This causes the user id to be reset every time the library loads

@duailibe
Copy link
Contributor Author

duailibe commented Jul 13, 2023

Added a failing test here so you can see the problem happening.

The fix should be as simple as removing the forward slash / from the beginning and the end of the regex

const regex = new RegExp("/(^|;)\\s*" + cookieName + "\\s*=\\s*([^;]+)/");

Since it's using the RegExp() constructor, those are escaped and will never match anything in document.cookie

@PabloReszczynski PabloReszczynski requested a review from sk- July 13, 2023 16:15
Copy link
Contributor

@sk- sk- left a comment

Choose a reason for hiding this comment

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

Thanks @duailibe for the PR and the report of the issue. The test you added correctly shows the issue you mentioned.

Would you be adding the fix you proposed in #182 (comment) ?

@duailibe
Copy link
Contributor Author

@sk- Pushed a commit with a fix. Let me know if there's anything else you need me to do

Thank you!

@sk-
Copy link
Contributor

sk- commented Jul 13, 2023

Thanks @duailibe, we will be releasing a new version including these changes soon.

@sk- sk- merged commit b807f92 into Topsort:main Jul 13, 2023
@duailibe duailibe deleted the fix-id-from-cookie branch July 13, 2023 17:08
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.

2 participants