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

fix: isBrowser() to include check on window #982

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

salmoro
Copy link
Contributor

@salmoro salmoro commented Nov 19, 2024

There are several checks in GoTrueClient.ts that check window?.[prop] after checking isBrowser() which throws an error in node environments that have document defined but not window.

Fixes:

@salmoro salmoro changed the title fix: isBrowser to include typeof window !== 'undefined' check fix: isBrowser() to include typeof window !== 'undefined' check Nov 19, 2024
@hf
Copy link
Contributor

hf commented Dec 6, 2024

What environment is this throwing in?

@salmoro
Copy link
Contributor Author

salmoro commented Dec 10, 2024

What environment is this throwing in?

Sveltekit.
It's so bad that the only work around we found was to create a pnpm patch on auth-js package and patches are major pain to maintain.

There are other situations this is an issue (see supabase/supabase-js#786) and many of those cases people are finding workarounds but not a root-cause fix.

There are several checks in GoTrueClient.ts that check window?.[prop] after checking isBrowser() which throws an error in node environments that have "document" defined but not "window". So this is the most minimal check to  be sure they don't throw by ensuring "window" is defined.
@j4w8n
Copy link
Contributor

j4w8n commented Dec 12, 2024

Just keep React Native, and possible other similar things, in mind here.

supabase/supabase-js#786 (comment)

@salmoro
Copy link
Contributor Author

salmoro commented Dec 12, 2024

Just keep React Native, and possible other similar things, in mind here.

supabase/supabase-js#786 (comment)

@j4w8n Yes. My changes here only add a requirement for isBrowser() to also check that typeof window !== 'undefined'. The previous check for typeof document !== 'undefined' is still there and will return false for React Native.

@hf hf changed the title fix: isBrowser() to include typeof window !== 'undefined' check fix: isBrowser() to include check on window Dec 16, 2024
@hf hf merged commit 645f224 into supabase:master Dec 16, 2024
4 checks passed
hf pushed a commit that referenced this pull request Dec 16, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.67.2](v2.67.1...v2.67.2)
(2024-12-16)


### Bug Fixes

* `isBrowser()` to include check on `window`
([#982](#982))
([645f224](645f224))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants