sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.71k stars 1.94k forks source link

Option to disable the @sveltejs/package warning about `import.meta.env` in the code #11414

Open Mooncat25 opened 10 months ago

Mooncat25 commented 10 months ago

Describe the problem

I'm using Vite + Svelte + @sveltejs/package to package Svelte components for my Svelte apps. One of the components needs to access window and therefore needs to check !import.meta.env.SSR to verify it is running on browser.

I'm not using SvelteKit because I don't need SSR.

The problem is, each time @sveltejs/package packages the component, it shows the warning: Avoid usage of `import.meta.env` in your code. It requires a bundler to work. Consider using packages like `esm-env` instead which provide cross-bundler-compatible environment variables.

Even though I'm actually using Vite as the bundler, and my Svelte apps are also using Vite. i.e. I would always use Vite for Svelte projects.

I noticed the warning was introduced in https://github.com/sveltejs/kit/pull/8922.

Describe the proposed solution

Allow disabling the warning by adding comment above the line that uses import.meta.env.SSR.

For example,

// @sveltejs/package-disable-warning
const isBrowser = !import.meta.env.SSR;

Alternatives considered

Before I realized I don't need SSR and SvelteKit, the component library actually used SvelteKit and $app/environment to check if it is running on browser. But I noticed the packaged component kept $app/environment and therefore would limit my app that uses the component to also use SvelteKit.

I have also tried using Vite's build command with its library mode but failed to figure out how to build Svelte components. I would be pleased if anyone could enlighten me to build Svelte component without @sveltejs/package.

Importance

nice to have

Additional Information

No response

benmccann commented 10 months ago

I guess we could remove this only when you have private: true set in your package.json. Though even then I'm not entirely sure we should because we can't know if an internally created package might be consumed by other bundlers. Is there a reason you don't just use esm-env as the warning suggests?

benmccann commented 10 months ago

I also noticed that the warning isn't quite accurate. I sent a PR to update it: https://github.com/sveltejs/kit/pull/11440

Mooncat25 commented 10 months ago

I guess we could remove this only when you have private: true set in your package.json. Though even then I'm not entirely sure we should because we can't know if an internally created package might be consumed by other bundlers. Is there a reason you don't just use esm-env as the warning suggests?

The idea of making it an option is to let package creators decide whether the warning is necessary. In my case, I am sure my packages will be consumed by Vite, because I only publish it in our internal registry for our Vite projects. Therefore I would like to disable the warning in my packages.

I don't use esm-env mainly because I don't know how it works. Installing another package for the same purpose that Vite already provides seems overkill to me. Or if you can explain to me that it doesn't increase build size and affect runtime performance, I will consider that.

Rich-Harris commented 10 months ago

Adding options is always a last resort, and it would be particularly unfortunate to add an option for this.

Or if you can explain to me that it doesn't increase build size and affect runtime performance, I will consider that.

Use the source — it will not increase build size, nor will it affect runtime performance. It will work in every project setup, and autocompletes more easily than import.meta.env. It's better in every respect.