Skip to content
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

Cannot find name 'require' and 'module' when @ts-check is on #21933

Closed
DanielRosenwasser opened this issue Feb 14, 2018 · 4 comments · Fixed by #23743
Closed

Cannot find name 'require' and 'module' when @ts-check is on #21933

DanielRosenwasser opened this issue Feb 14, 2018 · 4 comments · Fixed by #23743
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Feb 14, 2018

We spoke about this with @mjbvz a few months ago.

When tossing checkJs or // @ts-check into a project, users almost immediately get feedback like Cannot find name 'require' which is frustrating because we specifically understand how require works.

I understand that we want users to have more actionable feedback when they require a Node built-in, but the current behavior is not ideal.

@DanielRosenwasser
Copy link
Member Author

So two things:

  1. We now have Suggestion diagnostics where we can prompt people to npm install @types/node if these identifiers show up.

  2. We should ensure this works for

    • require
    • module
    • exports

@mhegazy mhegazy assigned sandersn and unassigned mhegazy Apr 25, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Apr 25, 2018

If we resolve the name require and it fails, and isRequireCall is true (i.e. it is a call expression, and it has a single argument that is a string literal), then we should resolve it to built-in require symbol (this should replace the use of isCommonJsRequire).

If this is a .ts file we should error like we do today, with an explanation Can not find name "require". Did you forget to install "@types\node"?.

If this is in a .js file, we should not, since we already treat these in a special way.

@sandersn
Copy link
Member

We should also special-case the built-in node modules and give an even better error message, like "Can not find module "fs". Did you forget to install @types/node? (This would also apply to JS.)

@mhegazy
Copy link
Contributor

mhegazy commented Apr 25, 2018

We have the list of built-in node modules in https://github.com/Microsoft/TypeScript/blob/master/src/services/jsTyping.ts#L34. we should use that in giving ppl better error message, and enabling a quick fix to install the app for you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants