Skip to content

Import nonexistent module is not checked #20767

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
evmar opened this issue Dec 18, 2017 · 11 comments
Closed

Import nonexistent module is not checked #20767

evmar opened this issue Dec 18, 2017 · 11 comments
Labels
Duplicate An existing issue was already created

Comments

@evmar
Copy link
Contributor

evmar commented Dec 18, 2017

TypeScript Version: playground

Code

import {a} from 'nosuchthing';  // error: Cannot find module

import 'othernonsense';  // no error, even with --noImplicitAny

Expected behavior:
Both of these should error. There should be a way to always get a compile-time error when you make a typo in an 'import' statement, regardless of which form you use.

Actual behavior:
No errors. I appreciate that it's intentional (in the implicit-any case) that modules lacking types are just treated as 'any', but that means in the case where you want to be more strict there's no way(?) to have TS check all module imports.

@RyanCavanaugh
Copy link
Member

Duplicate #20534

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Dec 18, 2017
@RyanCavanaugh
Copy link
Member

Note that you can write

import {} from 'othernonsense';

once #20671 is merged

@evmar
Copy link
Contributor Author

evmar commented Dec 18, 2017

Dang, sorry for the noise, I really did try to search for dup.

@evmar evmar closed this as completed Dec 18, 2017
@jamietre
Copy link

jamietre commented Jan 2, 2018

@RyanCavanaugh I'm confused. I posted #20534. You seem to be saying that we can use syntax import {} from 'something' as a workaround to ensure it will fail if something doesn't exist when importing no symbols, when this PR is merged.

According to the PR you reference, #20671 fixes a bug:

tsc should fail with an error "Cannot find module 'nothing'", just like what it does when we say
import 'nothing';
or
import {nothing} from 'nothing';
or
import * as nothing from 'nothing';

But you just told me in the issue I posted that not failing -- for the first syntax he describes -- "is the intended behavior as per many requests from users to support scenarios such as importing CSS modules"

Put another way: the pull request says *all these syntaxes should fail, and it's fixing a bug causing import {} ... to not fail."

You just told me in my bug report it's expected that the import 'something' syntax should not fail

Can you please clarify what is the expected behavior, and what it will be after this pull request is merged? It sounds like the author of the PR thinks that the syntax import 'something' will fail when something is not resolvable. However, obviously from my own issue and this one, that is not the case. So when this PR is merged, will the {} syntax fail, but the plain syntax not fail?

Again, I'm not sure why we'd want any of these to not fail, regardless of whether or not we choose to parse a module. But it seems that they should all be consistent; your comment here indicates that you think "{}" will be a workaround to the default behavior of not checking the module when importing no symbols, but clearly that's not what the author of the PR thinks (he thinks they should be consistent).

@mhegazy
Copy link
Contributor

mhegazy commented Jan 4, 2018

Please see my response in #20576 (comment)

@jamietre
Copy link

jamietre commented Jan 4, 2018 via email

@jamietre
Copy link

jamietre commented Jan 4, 2018 via email

@mhegazy
Copy link
Contributor

mhegazy commented Jan 4, 2018

Why can't the existence of a file be checked without parsing it? Wouldn't
people importing css or anything else still want a build failure if the
module is missing?

it would. but that means the compiler needs to monitor mode files, and resolve more files, module resolution is an expensive operation, and ideally we would not have to do it.

it becomes more interesting when users have module loader extensions, e.g. requirejs: http://requirejs.org/docs/plugins.html (foo!something.css) vs. SystemJs plugins which have a diffrent name.

@jamietre
Copy link

jamietre commented Jan 4, 2018

One other thought:

While I understand why (some) people want this, ultimately importing javascript is actually a real, valid use of import, while importing CSS/webpack loaders/etc is a non-compliant overload.

Why not create a special directive people can use for the non-spec case, which is compiled to import if es6 or require if commonjs?

(I realize this would be a big breaking change - but again - a flag to opt in)

@mhegazy
Copy link
Contributor

mhegazy commented Jan 4, 2018

Why not create a special directive people can use for the non-spec case, which is compiled to import if es6 or require if commonjs?

We really do not want to add any new non-ES-complaint syntax. we only do it when we absolutely have to.

(I realize this would be a big breaking change - but again - a flag to opt in)

there is a cost for adding additional flags both in complexity and maintenance.

@jamietre
Copy link

jamietre commented Jan 4, 2018

I understand that it's a thing to be done. But isn't supporting the actual spec usage at least as important as supporting non-spec usage? Even a simple flag to enable/disable import-less imports would be of immense value. I think it's far more common these days to use webpack config to map extensions to loaders than to inline the config on imports. We use webpack too, and we'd really like import 'path/to/something.css' to be validated (as well as the occasional bootstrapping module). I guess this could be done with a post-build process, but what a lot of work to do something that the compiler is already designed to do.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants