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

Make SSR environment-agnostic (don't count on 'global') #3561

Closed
warmrobot opened this issue Sep 12, 2019 · 8 comments · Fixed by #4628
Closed

Make SSR environment-agnostic (don't count on 'global') #3561

warmrobot opened this issue Sep 12, 2019 · 8 comments · Fixed by #4628

Comments

@warmrobot
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We do not use Node, we use v8 instance. So, when I compile simple test page in SSR mode, I get 'Uncaught ReferenceError: global is not defined'.

Describe the solution you'd like
Add typeof global check here

export const globals = (typeof window !== 'undefined' ? window : global) as unknown as typeof globalThis;

If global is undefined then return {}

How important is this feature to you?
No chances, that we start using Node because of this. (
So, I had to add global mock (somehow), but better solution is to change the line, that I mentioned

Additional context
изображение

@Conduitry
Copy link
Member

Changing it to export {} if global is not defined will not work in general, because we do sometimes refer to globals.thing instead of referring directly to thing. Is there some other way you have access to an object of all globals?

In any case, I do think that shimming global might be your best bet. This seems kind of niche. In javascript environments that are not browsers, I'm sure Node is the most popular by a wide margin.

@Conduitry Conduitry added the awaiting submitter needs a reproduction, or clarification label Sep 12, 2019
@warmrobot
Copy link
Contributor Author

warmrobot commented Sep 12, 2019

Of cource, NodeJS is most popular, but environment-agnostic is a good thing. Vue team, for example, take care of it since version 2.5
https://ssr.vuejs.org/guide/non-node.html
And yes, they recommend shiming.

What do you mean about access to an object of all globals? This is NodeJS only feature, not v8:
https://nodejs.org/docs/latest/api/globals.html

@Conduitry
Copy link
Member

Consider the following component, compiled for SSR:

<script>
	let Object;
</script>

<Foo {...foo}></Foo>

Svelte uses Object.assign for spread properties in SSR mode. If there's another variable local to the component that shadows that, we still need access to the global Object anyway. We do this by importing the globals object and then doing const { Object: Object_1 } = globals; and using Object_1.assign instead.

Shimming global/globals as {} might work in your code if you're careful not to use these names, but that's not something we can do in general. We need some escape hatch to access the global Object (for example) even if it's been shadowed.

@warmrobot
Copy link
Contributor Author

Ok, I see.
How about to add globalThis to your globals assignment? It is supported in latest v8
Problem will be solved.

@Conduitry
Copy link
Member

I think that would work, although we'd still need to fall back to global, because we want to support versions of Node before 12.

@Conduitry Conduitry added feature request and removed awaiting submitter needs a reproduction, or clarification labels Sep 12, 2019
@warmrobot
Copy link
Contributor Author

Yes. And may be check for this and the very end. Like this way https://v8.dev/features/globalthis
Thank you!

@stefan-pdx
Copy link

Hello! I came across this issue in #4545. (Please take a look at it if you get a chance... I don't understand why internal/globals.ts is being pulled into the Rollup build when importing from svelte/store)

If we implemented a technique like @warmrobot, we'd be able to support using Svelte stores in WebWorkers which allow for reactive programming (via store subscriptions) in that environment.

@Conduitry
Copy link
Member

Released in 3.21.0 - we now try globalThis before trying global.

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

Successfully merging a pull request may close this issue.

3 participants