$app/environment.browser isn't reliable for libraries (packaging)

[Tal500]

Describe the bug

Every Svelte UI library that want to check if the current environment is the server or the client, uses often the variable $app/environment.browser.
While it's the recommended way for SvelteKit normal application, this is problematic when the library do svelte-kit package, to get the library compiled output.

Any project using a UI library that written in SvelteKit, exported via svelte-kit package, and use the browser variable, will fail to be used in Svelte projects that are outside the SvelteKit environment (e.g. in REPL), since the output still use the browser variable, and wait for the environment to compile it.

There are two possible solutions:

1. Put the browser variable as part of the specific Svelte environment, instead of the specific SvelteKit environment. (Recommended)
2. When compiled via svelte-kit package, define the value of browser to be equal to typeof window !== 'undefined'.

Sadly, I've seen too many libraries do this mistake, and proposed them to use typeof window !== 'undefined' instead. However, the logic and the sanity are on their side :-)

Reproduction

Just use the browser definition, export via svelte-kit package and see the output isn't compiled at all, the browser stays in the output.

Logs

No response

System Info

N/A

Severity

annoyance

Additional Information

No response

[benmccann]

It's defined as !import.meta.env.SSR, but if you're not using Vite that won't be available by default, which I suppose can be non-ideal in some circumstances. I'm not sure that typeof window !== 'undefined' would be equivalent because I think it'd only get evaluated at runtime and not buildtime.

I'm not super familiar with the REPL, but it looks like it calls the compiler directly: https://github.com/sveltejs/sites/blob/master/packages/repl/src/lib/workers/bundler/index.js

I wonder if we should define import.meta.env in rollup-plugin-svelte and have the REPL use rollup-plugin-svelte?

[Tal500]

I'm not sure that typeof window !== 'undefined' would be equivalent because I think it'd only get evaluated at runtime and not buildtime.

Do you think it wouldn't be inlined even after optimization&minimization?

I'm not super familiar with the REPL, but it looks like it calls the compiler directly: https://github.com/sveltejs/sites/blob/master/packages/repl/src/lib/workers/bundler/index.js

I'm not familiar with REPL internal as well, I have just seen this bug in libraries over and over again when using REPL or Sapper(which although obsolete it proves that it wouldn't work outside SvelteKit) for example.

I wonder if we should define import.meta.env in rollup-plugin-svelte and have the REPL use rollup-plugin-svelte?

Is it problematic to have some environment variables in the Svelte language by itself?

[TorstenDittmann]

Not sure if related, but after upgrading to kit version next.409, I've been running into following issues with vitetest.

[brev]

@TorstenDittmann I think you are seeing similar to Issue #6259, please see my comment there for a (temporary?) solution.

edit: sorry, Issue #6259

[benmccann]

closing as duplicate of #1950

[Tal500]

closing as duplicate of #1950

I understand the relationship, but I don't understand why it's duplicated?

Assuming #1950 will be solved, what will you substitute under environment.browser during packaging? It's determinated on the user compilation step, if it's on SSR or not.

[benmccann]

Ah, yeah, I suppose this one is a bit different than the rest

I think you'd need to bundle it twice, once with import.meta.env.SSR set to true and one with it set to false and then use the exports map to export one browser version and one default/server version.

[Tal500]

I think you'd need to bundle it twice, once with import.meta.env.SSR set to true and one with it set to false and then use the exports map to export one browser version and one default/server version.

That's not a great method in my opinion, because:

1. As far as I understand, Vite (unlike in Sapper, thankfully) doesn't compile from scratch one pass for browser and one for SSR. It transforms, using plugins, the code to JS chunks, and then bundle them together once for server and once for client, so separate library bundling won't work in my opinion.
2. That's sounds crazy for the user and the library maintainer no? In my opinion as a library writer I'd rather use typeof window !== undefined. But it's not fair because I know this internal points, while most library creators don't(and how you'd assume they will? unless it will be written as an explicit warning in SvelteKit packaging docs).

You can decide that on Vite and Rollup plugin of svelte, there will be defined the "environment" like SvelteKit.
However, I think that the robustest solution is to add some form of mini-environment to svelte itself, something like svelte/environment.

[benmccann]

As far as I understand, Vite (unlike in Sapper, thankfully) doesn't compile from scratch one pass for browser and one for SSR.

That's incorrect. The SSR and browser builds are entirely separate invocations of Vite

That's sounds crazy for the user and the library maintainer no? In my opinion as a library writer I'd rather use typeof window !== undefined

That's worse for the user as it results in larger bundle sizes since it prevents dead code removal, etc. Using the browser condition in an exports map would be transparent to the user

[Tal500]

I have two ideas how we may solve this problem and makes everyone happy, and they are both using a similar idea that raises an some issue, so I'll first say the idea and then will describe the concrete solution.

Idea

During the packaging process,

1. Replace any occurrence of browser to be !import.meta.env.SSR, as it already defined here:
   

   kit/packages/kit/src/runtime/app/environment.js

   Lines 1 to 4 in d0b51ae

   	/**
   	* @type {import('$app/environment').browser}
   	*/
   	export const browser = !import.meta.env.SSR;

   
   Any usage of other environmental data will raise an error to the package developer that it's not safe to refer this data. (A breaking change, sorry).
2. This way, a user outside of Vite (e.g. Rollup) may define (e.g. by rollup-define plugin) the environment variable import.meta.env.SSR, and use the library in a correct way.
3. We can have somehow a "fallback solution" when the value of the environment variable isn't defined (a totally legitimate case in my opinion) - in this case use the non-optimized solution of resolving the value of typeof window === 'undefined'.

Sadly for implementing 3, the problem in this solution is that import.meta.env.SSR will raise error in the case that this was not defined (e.g. outside Vite), since in this case import.meta.env === undefined.

Concrete Solutions to The Problem

Here are 4 possible concrete implementation of the idea that I believe are friendly to minification (i.e. resolving the browser value in the build time), each of them has pros and cons.
i. Create a new token to SvelteKit, something like __SVELTE_KIT_SSR, specifying if we're on SSR or not. The value of it could be defined in Vite.config.define during the config-resolved step of the Kit plugin of Vite. Why is this better than using import.meta.env.SSR? Since if the value of it wasn't defined, we'll get __SVELTE_KIT_SSR === undefined instead of an error. Sadly, this is a breaking change (meaning that both library developers and users should use an upgraded SvelteKit version).
ii. Surround the resolving process of import.meta.env.SSR with a try and catch block, and . I believe all "normal" browsers will raise an error if the value of import.meta.env isn't available. I.e. something like:

const browser = ( () => {
  try {
    return import.meta.env.SSR ?? (typeof window !== 'undefined');
  } catch (e) {
    return (typeof window !== 'undefined');
  }
});

Didn't test it, but it seems to be easy to be minified as well.
iii. It is hinted by Vite documentations as a side-effect(not a feature), that the values are being replaced also on strings. So we may check first if "import.meta.env.SSR" equals to "true" or "false". Don't know for sure however if constant strings compering operations are being minified.
iv. Read import.meta.env?.SSR and then the fallback, i.e.:

export const browser = import.meta.env?.SSR ?? (typeof window !== 'undefined');

This will work well everywhere, and will be known at compile time only on Vite. The issue is that defining import.meta.env.SSR on rollup for example (outside of Vite) will not work(as far as I know), because that the exact syntax is import.meta.env?.SSR.

[Tal500]

Additional suggestion before releasing 1.0(assuming we're going in the path I mentioned), so there will be less breaking changes, one of the two:

1. Take the browser variable outside of $app/environment, so it won't conflict with the other variables during the library packaging.
2. Define the type of the rest of varaibles of $app/environment (e.g. if the build is production) to accept the possible value of undefined, which can happen in the library packaging mode.

[Tommertom]

Hi there

I need import { beforeNavigate } from '$app/navigation'; in my library as this hook is to ensure the UI elements in the library and its consumers can handle logic relevant to the page navigation. (https://github.com/Tommertom/svelte-ionic-npm/blob/main/components/IonPage.svelte)

Will the solution you are working on for 1.0 (@Tal500 ) make it possible to use this module in a package as well?

Otherwise, I'd like to open a new issue on this part.

ps. this library is to enrich the ecosystem with Svelte magic powered stuff - so to stay as closely as possible to this library in terms of the API compared to React/Vue/Angular, I really need this svelte-module to work.

ps2. I did try to reference the navigation package via @sveltejs/kit/src/runtime/app/navigation or with .js, but that did not work either. Probably because it is something in runtime?

[Tal500]

Hi there

I need import { beforeNavigate } from '$app/navigation'; in my library as this hook is to ensure the UI elements in the library and its consumers can handle logic relevant to the page navigation. (https://github.com/Tommertom/svelte-ionic-npm/blob/main/components/IonPage.svelte)

No way that the navigation part will be part of Svelte core, so this is too much to ask. However, I can suggest you two simple logical solutions to your case(you can use both of them together!):

1. Export a function that act in navigation from your library, and ask the user to call this function whenever his website navigates. Tell the user he can simply do this by register to the SvelteKit event, by giving him a code sample. If he doesn't uses SvelteKit, he can still call this function by himself.
2. Separate the navigation calling to a different source file, and tell the user that if he uses SvelteKit, he may import this file and the navigation will works well for him. If he doesn't uses SvelteKit, he must not import this file, but rather do something like described in 1.

Will the solution you are working on for 1.0 (@Tal500 ) make it possible to use this module in a package as well?

I am not currently working on a solution, I was only throwing suggestions to the air.

ps2. I did try to reference the navigation package via @sveltejs/kit/src/runtime/app/navigation or with .js, but that did not work either. Probably because it is something in runtime?

The $app/navigation is generated by SvelteKit to every project (as well of the rest of $app), so you must import this path only, if you want at all.

Will be happy to suggest more things if you tag me on an issue you'll open on your Github project.

[Tommertom]

Hi @Tal500 -

thx for this - then I know what to do for the library. Not a huge loss - but good to know and inform the developer to be using beforeNavigate themselves instead of relying on the API from the documentation.

[benmccann]

Closing in favor of #8033. Please see the recommended solution there