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

[Doc] Add global definition to the Vite example, and offer to install the Roboto font locally #8680

Merged
merged 2 commits into from
Feb 24, 2023

Conversation

slax57
Copy link
Contributor

@slax57 slax57 commented Feb 24, 2023

  • Adding window.global = window to the Vite example allows to fix errors with some libs, e.g. react-floater, that would throw errors such as Uncaught ReferenceError: global is not defined
  • Offering to install the Roboto font locally can help with local legislation compliance, such as the GDPR in Europe

@slax57 slax57 added the WIP Work In Progress label Feb 24, 2023
@djhi djhi added this to the 4.8.2 milestone Feb 24, 2023
@djhi djhi merged commit fa3a856 into master Feb 24, 2023
@djhi djhi deleted the vite-doc-global branch February 24, 2023 09:38
@smeng9
Copy link
Contributor

smeng9 commented Feb 26, 2023

We should add a troubleshoot note to user that if they encounter Uncaught ReferenceError: global is not defined they should submit a bug report to the author of the library they are using.

Adding a window.global is the last resort as it may change the behavior of some other library that detects node environment based on whether global is defined or not.

@slax57
Copy link
Contributor Author

slax57 commented Feb 27, 2023

@smeng9 I wasn't aware of that, thanks for letting us know!
Do you have any resources explaining a bit further why this could be a bad practice, and how the library maintainers should rather solve the issue?
Thanks.

@smeng9
Copy link
Contributor

smeng9 commented Feb 27, 2023

Hi @slax57

You can refer to npm docs about the recommended way library maintainer should do
https://docs.npmjs.com/cli/v9/configuring-npm/package-json#browser
to make the the package usable in both node and browser environment.

If package is set up correctly, bundlers will resolve using the browser field entry for a copy of the library that does not use global in browser, and main field for a copy of the library that does not use window in node.

However some of the packages do not follow this recommended path and only have a single main entry. Instead, they check existence of certain variables (like global), and may access file systems (which are not available inside browser). Adding a window.global may make some of the code that should only be executed in node environment to be executed in browser environment and such behavior may cause other side effects.

@djhi
Copy link
Collaborator

djhi commented Mar 2, 2023

It seems this issue does not happen anymore, probably because our dependencies have been updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants