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

fix: added syntactic and semantic diagnostics #310

Merged
merged 1 commit into from Mar 20, 2024

Conversation

gcaaa31928
Copy link
Contributor

No description provided.

@qmhc
Copy link
Owner

qmhc commented Mar 18, 2024

What's the motivation for this change?

In buildStart hook, the plugin uses program.getDeclarationDiagnostics() to get all diagnostics info.

@gcaaa31928
Copy link
Contributor Author

Apologies for the lack of explanation.

If we encounter the incorrect statement export const msg: string = 'hello' as number; in the TypeScript file, it can lead to unexpected results in the generated bundle. For instance, we might observe the declaration in the generated .d.ts file as export declare const msg: string;, which might not align with our intended outcome.

@qmhc
Copy link
Owner

qmhc commented Mar 18, 2024

Emm, I don't understand is that why you need them to use although program.getDeclarationDiagnostics() is run.

Doesn't program.getDeclarationDiagnostics() catch the error if export const msg: string = 'hello' as number; threw?

@gcaaa31928
Copy link
Contributor Author

gcaaa31928 commented Mar 18, 2024

Yes, getDeclarationDiagnostics won't catch this error, you can test it on example

@qmhc
Copy link
Owner

qmhc commented Mar 19, 2024

Well, I think you can change const diagnostics = program.getDeclarationDiagnostics() (line 368) to the following instead of current:

const diagnostics = [
  ...program.getDeclarationDiagnostics(),
  ...program.getSemanticDiagnostics(),
  ...program.getSyntacticDiagnostics()
]

@gcaaa31928
Copy link
Contributor Author

Well, I think you can change const diagnostics = program.getDeclarationDiagnostics() (line 368) to the following instead of current:

const diagnostics = [
  ...program.getDeclarationDiagnostics(),
  ...program.getSemanticDiagnostics(),
  ...program.getSyntacticDiagnostics()
]

Sure! already push and changed it, please review it

@qmhc qmhc merged commit 7c10782 into qmhc:main Mar 20, 2024
4 checks passed
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.

None yet

2 participants