sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.18k stars 4.09k forks source link

Allow defining global to avoid 'FOO is not defined' warnings #7240

Closed Prinzhorn closed 5 months ago

Prinzhorn commented 2 years ago

Describe the problem

I'm injecting globals using esbuild (https://esbuild.github.io/api/#define) and am flooded with warnings.

Describe the proposed solution

Allow configuring globals in addition to the hard coded list https://github.com/sveltejs/svelte/blob/5665f711fd699d8b7d2e07524b19cab194951d11/src/compiler/compile/Component.ts#L1445

I'm already defining them in my eslint globals config but it appears there is no such things for Svelte?

Alternatives considered

  1. Sprinkling <!-- svelte-ignore missing-declaration --> all over the place. Depending on how fine grained I (can) do this it might mask legit warnings (the comment applies to the entire element and subtree, while I only want a specific thing to be known)
  2. Instead of injecting sth. like MY_GLOBAL I could hack around and use window.MY_GLOBAL. This gets rid of the warning but introduces a new problem: if esbuild fails to replace this would leave the literal window.MY_GLOBAL in the code (which would be undefined).

Importance

would make my life easier

dummdidumm commented 2 years ago

Another alternative would be to use the onwarn property that I believe rollup/vite/webpack-plugin-svelte support. It expects a function which you can use to filter out false positive warnings.

Prinzhorn commented 2 years ago

Another alternative would be to use the onwarn property that I believe rollup/vite/webpack-plugin-svelte support. It expects a function which you can use to filter out false positive warnings.

Thanks, I did stumble across onwarn in some related issues but didn't find any docs. If it's not a Svelte feature then this explains why. And I guess https://github.com/EMH333/esbuild-svelte/issues/85 settles that.

Edit: this also came up before in the rollup plugin repo https://github.com/sveltejs/rollup-plugin-svelte/issues/179

baseballyama commented 2 years ago

https://github.com/sveltejs/svelte/pull/7786 registered all global objects / functions.

Prinzhorn commented 2 years ago

@baseballyama this issue is about defining my own globals (equivalent to the eslint globals config), please re-read my original post.

baseballyama commented 2 years ago

Ahh 😩😩😩 Sorry for the misunderstanding.

brunnerh commented 1 year ago

Does this issue address existing type declaration files not being used?

E.g. if I have a vite.d.ts file:

/**
 * If this is a debug build.  
 * Constant is defined in `vite.config.js` and is replaced everywhere automatically.
 */
declare const DEBUG: boolean;

Interestingly the language tooling picks up on it, but you still get the warning:

image

duydang2311 commented 1 year ago

I am also experiencing this false positive warning. This variable is declared in vite-env.d.ts and then defined in entry file main.ts.

// vite-env.d.ts
declare var PageHash: {
    SignUp: 'signup';
    Login: 'login';
};

// main.ts
globalThis.PageHash = {
    SignUp: 'signup',
    Login: 'login',
};

image

The language server complains but the compiler is still able to pick up the global variable.

Oh well, the language server is good if I explicitly access PageHash through globalThis object, like globalThis.PageHash.

kobe-ra commented 1 year ago

Would be pretty cool if we could do this for example. Or just provide List of global functions.

image

dummdidumm commented 1 year ago

Part of me wonders if we still need this warning at all in 2023. TypeScript/typed JavaScript is ubiquitous at this point, which does a much better job at knowing which globals exist.

Rich-Harris commented 5 months ago

We got rid of this warning in Svelte 5 for that exact reason, so I'll close this