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

isServer detection faulty when jsdom is present #154

Open
merk opened this issue Dec 6, 2023 · 5 comments
Open

isServer detection faulty when jsdom is present #154

merk opened this issue Dec 6, 2023 · 5 comments
Labels
PRs Accepted Feel free to pick this up and make a PR

Comments

@merk
Copy link

merk commented Dec 6, 2023

My testing environment installs jsdom which sets a window global - meaning that all tests run as a client according to t3-env.

My initial reaction is to override the isServer check to something like this solution mentioned on the jsdom repo.

It feels like an acceptable solution (and I'm happy to send a PR) but I'm not 100% sure if the change is a good one?

https://github.com/t3-oss/t3-env/blob/63a7fed1cb85e05d59c1c3c87e5b434cc8a353cc/packages/core/index.ts#L202C37-L202C66

@chungweileong94
Copy link
Contributor

This actually make t3-env support Deno out of the box, so why not🤷

The only question is how reliable is this Object.getOwnPropertyDescriptor(globalThis, 'window')?.get?.toString().includes('[native code]') ?? false, but it works pretty good on environment like NodeJS, Bun, Deno, Arc(Chromium), Safari.

@juliusmarminge
Copy link
Member

I'd say this is an app-concern and you should use the option to override the default behavior. We can't cover every case by default and I think typeof window === "undefined" is the most well-known and used option out there

@juliusmarminge juliusmarminge closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2024
@juliusmarminge
Copy link
Member

Hmm looks like we use a slightly more covering one in trpc so maybe?

https://github.com/trpc/trpc/blob/next/packages/server/src/unstable-core-do-not-import/rootConfig.ts#L18

@juliusmarminge juliusmarminge added the PRs Accepted Feel free to pick this up and make a PR label Mar 24, 2024
@juliusmarminge
Copy link
Member

juliusmarminge commented Mar 24, 2024

Feel free to extend the default isServer check to something that covers more runtimes. We can't cover everything though so it has to be a reasonable default for common setups widely used

@michaellopez
Copy link
Contributor

I added detection of Deno as server in #220 (Netlify uses Deno to bundle Next.js for edge)

For those coming here for a workaround until #220 is merged can add this to createEnv(...) call for server environment:

isServer: typeof window === "undefined" || "Deno" in window,

e.g.

export const env = createEnv({
  isServer: typeof window === "undefined" || "Deno" in window,
  server: {
    ...
  }
  ...

michaellopez added a commit to weahead/t3-env that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PRs Accepted Feel free to pick this up and make a PR
Projects
None yet
Development

No branches or pull requests

4 participants