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

Non browser environments fix #103

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

nr-tw
Copy link
Contributor

@nr-tw nr-tw commented Nov 11, 2022

  • Fixed an issue with isNodeEnvironment which does not work in environments which have removed the window object
  • Brought back an older method for identifying node environment as it's needed to run the examples in an "ssr-like" environment
  • Fixed an issue where v8-snapshot would fail due to an early setTimout access.

@@ -1,6 +1,6 @@
{
"name": "@react-loosely-lazy/manifest",
"version": "1.0.0",
"version": "1.1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't bump version in this PR

return true;
}

return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we keep globalThis !== globalThis.window as value of last return? If something else provides/sets a window key in global we might still falsely believe we are in a browser env.

*
* @see https://github.com/jsdom/jsdom/issues/1537
*/
export const isNodeEnvironment = () => {
return globalThis !== globalThis.window;
// Some SSR environments remove the window object
if (typeof window === 'undefined') {
Copy link
Collaborator

@MonicaOlejniczak MonicaOlejniczak Nov 14, 2022

Choose a reason for hiding this comment

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

Can you use document instead? That's what SSR expects and is used by other libraries like emotion and should cover jsdom

return true;
}

// Official way to check for JSDOM
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that documented somewhere? Also we should treat jsdom as a browser, and return false

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