Skip to content

No error for non-existing css side effect import with noUncheckedSideEffectImports flag on #59990

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

Closed
dimafirsov opened this issue Sep 17, 2024 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@dimafirsov
Copy link

dimafirsov commented Sep 17, 2024

πŸ”Ž Search Terms

noUncheckedSideEffectImports

πŸ•— Version & Regression Information

⏯ Playground Link

https://stackblitz.com/edit/vitejs-vite-v2boec?file=src%2Fmain.ts

πŸ’» Code

import 'does-not-exist.css';

πŸ™ Actual behavior

With the noUncheckedSideEffectImports ts compiler flag on, the error is not thrown for the non-existing .css file reference (side effect import) when executing tsc. Actually, it is not thrown for any file extension other than js.

πŸ™‚ Expected behavior

As stated here, the error should be thrown for non-existing css file/module reference.

Additional information about the issue

Steps to reproduce:

  1. Visit the playgound typescript project
  2. Expect the runtime error (because of the non-existing file reference in the code)
  3. main.ts - notice the side effect import of non-existing CSS file
  4. globals.d.ts - notice the file and its contents (as described HERE)
  5. Open new stackblitz terminal
  6. Execute the following: npm run check
  7. Observe a result

Expected result: tsc throws an error for the non-existing file reference

Actual result: Specifically, the tsc successfully finishes with no errors. In general, tsc doesn't throw no matter the file extension, EXCEPT for the .js

@uhyo
Copy link
Contributor

uhyo commented Sep 19, 2024

You cannot benefit from --noUncheckedSideEffectImports while using ambient module declaration.

declare module '*.css' in your globals.d.ts is the way to have TS assume every module specifier that ends with .css is valid.

Mention to .css files in the 5.6 release announcement is all about use cases not covered by --noUncheckedSideEffectImports.

If you want to utilize --noUncheckedSideEffectImports for CSS imports, you need to stop using ambient module declarations and instead use a solution like this that generates .d.ts files next to each of your .css files

@dimafirsov
Copy link
Author

dimafirsov commented Sep 19, 2024

You cannot benefit from --noUncheckedSideEffectImports while using ambient module declaration.

declare module '*.css' in your globals.d.ts is the way to have TS assume every module specifier that ends with .css is valid.

Mention to .css files in the 5.6 release announcement is all about use cases not covered by --noUncheckedSideEffectImports.

If you want to utilize --noUncheckedSideEffectImports for CSS imports, you need to stop using ambient module declarations and instead use a solution like this that generates .d.ts files next to each of your .css files

Thank you for your response and for the alternative options you gave that may make things work.

However though, it is either a confusing wording in release announcement, or it is indeed an issue.

The release announcement for this section literally reads like this:

  1. Some people may have their builders configured to directly import .css files
  2. this masks potential typos on side effect imports
  3. That’s why TypeScript 5.6 introduces a new compiler option

A bit below we can read this:

  1. When enabling this option, some working code may now receive an error, like in the CSS example above.
  2. To work around this, users who want to just write side effect imports for assets might be better served by writing what’s called an ambient module declaration with a wildcard specifier

Not a word about the flag not supporting errors for .css or any other side effects import modules.

@dimafirsov
Copy link
Author

Another thing to add to the conversation here is the name of the flag (noUncheckedSideEffectImports ) is pretty general, which makes me think it covers all the side effect imports, not just js modules.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 19, 2024
@RyanCavanaugh
Copy link
Member

I don't really understand what you're proposing happen here. There still needs to be a mechanism to tell TypeScript about modules which will successfully resolve but aren't on disk. That mechanism is declare module, and you used declare module. TypeScript correctly interpreted your description of the world (even if it's not what you meant to describe).

@dimafirsov
Copy link
Author

dimafirsov commented Sep 20, 2024

I don't really understand what you're proposing happen here. There still needs to be a mechanism to tell TypeScript about modules which will successfully resolve but aren't on disk. That mechanism is declare module, and you used declare module. TypeScript correctly interpreted your description of the world (even if it's not what you meant to describe).

Thanks @RyanCavanaugh for joining the discussion.

Ok, it'll put it in other words. My recent testing has shown that the following side effect imports (and their variations) are well-treated by typescript and it throws an expected error (with the compiler flag on, of course):

import 'does-not-exit'
import 'does-not-exit.does-not-exit''
import 'does-not-exit.1234567'
import '1234567.1234567'
etc.

However, once there is .css in the file name (or anything css-related, like less, sass etc) ts ignores such imports, whether there is declare module '*.css' {} or not.

import 'does-not-exit.css'

I wonder why ts reacts to the .css in the name this way?

Having .css in the name of the file may not necessary mean it is a css file. Imagine a rough use case when one has a js module file that exports some stuff that generates some CSS (the reason to have css in the name). Something like *.css.js. It'll be ignored when imported incorrectly even though its a js module, unless the .js extension is specified explicitly.

@RyanCavanaugh
Copy link
Member

I can't reproduce what you're reporting there.

D:\Throwaway\nusi>tsc
a.ts:1:8 - error TS2307: Cannot find module 'foo.css' or its corresponding type declarations.

1 import "foo.css";
         ~~~~~~~~~

Maybe you have a @types library that also does declare module "*.css" ?

@dimafirsov
Copy link
Author

dimafirsov commented Sep 21, 2024

Good point @RyanCavanaugh , I think I know what's going on there. Even though the environment for the reproduction I gave was clean, it was, apparently, built by Vite. And Vite does this:

image

And a lot more.
I didn't know or expect that, and now everything makes total sense.

So, thank you @uhyo and @RyanCavanaugh for your input, and I'm sorry for the false alarm. Closing this one.

@dimafirsov
Copy link
Author

Not an issue. Works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants