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

feat(diagnostic): add missing error codes #177

Merged
merged 10 commits into from
Mar 9, 2023

Conversation

tommy-mitchell
Copy link
Contributor

@tommy-mitchell tommy-mitchell commented Mar 7, 2023

Fixes #170. Fixes #143.

Adds missing diagnostic codes:

  • ts2375, ts2379, and ts2412, which occur when exactOptionalPropertyTypes is set to true in tsconfig.json.
  • ts2542, which occurs when attempting to modify a readonly key in an object.

Related, I think it's worth improving the error reporting when expectError fails. Currently, it reports

Expected an error, but found none.

even if another error is reported for the same invocation (i.e., tsd doesn't support that diagnostic code). We could change it to:

Expected an error, but found none. tsd may not currently support this error code, consider creating an issue on GitHub.

I used the following snippet in compiler.ts to confirm which DiagnosticCode to add1:

if (ignoreDiagnosticResult === 'preserve') {
  console.log(diagnostic.code);
}

I think it would be worthwhile to either report the codes of all diagnostics (potentionally in a --verbose mode?), or at the very least in the expectError message:

let message = 'Expected an error, but found none.';

if (diagnostic.code) {
  message += ` tsd may not currently support \`ts${diagnostic.code}\`, consider creating an issue on GitHub.`;
}

Footnotes

  1. Full list here.

@tommy-mitchell
Copy link
Contributor Author

Perhaps users could set additional diagnostic codes in their tsd config, to let their tests still pass if that code isn't in our list.

@tommy-mitchell tommy-mitchell changed the title feat(diagnostic): add codes for exactOptionalPropertyTypes: true feat(diagnostic): add missing error codes Mar 8, 2023
@tommy-mitchell
Copy link
Contributor Author

Added ts2542 to fix #143.

@sindresorhus
Copy link
Collaborator

or at the very least in the expectError message:

👍

@sindresorhus
Copy link
Collaborator

or at the very least in the expectError message:

Do you plan to work on this? No pressure, but I need to know whether or not to release a new version after merging this.

@sindresorhus sindresorhus merged commit 168de40 into tsdjs:main Mar 9, 2023
@tommy-mitchell
Copy link
Contributor Author

I did start attempting to add the updated error message, but it‘s a little more involved because the diagnostic object at that location doesn’t have access to the error code.

Should we just update the error message and add the code there, or report the code on all diagnostics?

@sindresorhus
Copy link
Collaborator

It would be too noisy to have the code on all diagnostics.

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

Successfully merging this pull request may close these issues.

expectError finds none, and finds an error... for the same invocation. Not catching readonly error
2 participants