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

Enable checkJs to switch opt-in to opt-out for tsc #4885

Open
turadg opened this issue Mar 21, 2022 · 3 comments
Open

Enable checkJs to switch opt-in to opt-out for tsc #4885

turadg opened this issue Mar 21, 2022 · 3 comments
Assignees
Labels
tooling repo-wide infrastructure

Comments

@turadg
Copy link
Member

turadg commented Mar 21, 2022

What is the Problem Being Solved?

Many files in the repo aren't type checked at all because they don't have @ts-check and our tsc compiler options are just to allowJs (implied by using jsconfig.json).

Description of the Design

Enable checkJs option (ref) which "is the equivalent of including // @ts-check at the top of all JavaScript files which are included in your project".

Note that this is blocked on #4560 because anything greater than the default maxNodeModuleJsDepth of zero means enabling checkJs will make tsc check inside the node_modules directory.

Security Considerations

--

Test Plan

--

@turadg turadg added the tooling repo-wide infrastructure label Mar 21, 2022
@turadg
Copy link
Member Author

turadg commented Nov 3, 2022

More more motivation: the @ts-check makes js apps that have SDK packages in node_modules re-check them. I haven't devised a way to prevent this other than patching the packages: Agoric/wallet-app@7cad807

@turadg turadg self-assigned this Nov 3, 2022
@mhofman
Copy link
Member

mhofman commented Nov 3, 2022

Weird, I didn't think tsc reported issues in node_modules path. Also to clarify, I suppose this wouldn't be a problem if all SDK packages had correctly exported types using generated .d.ts files?

Aka if we solved #6343

@turadg
Copy link
Member Author

turadg commented Nov 3, 2022

Weird, I didn't think tsc reported issues in node_modules path

Example:

Errors  Files
     4  ../../node_modules/encoding/lib/encoding.js:59
    60  ../../node_modules/node-fetch/lib/index.js:3
     4  src/main.js:4

Many have more than that.

I suppose this wouldn't be a problem if all SDK packages had correctly exported types using generated .d.ts files?

I think that's right, because the .d.ts files would be read instead. But I'm not certain because I think I've seen some Endo packages that were converted over still have the problem. e.g. I think iI saw an error forbundle-source, though that's probably due to SDK being on the latest release (not dev) which lacks the types builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling repo-wide infrastructure
Projects
None yet
Development

No branches or pull requests

3 participants