-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Switches the bundlePackageName error message to be a suggestion, and not a compiler error #41510
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
Conversation
…not a compiler error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good, low-risk compromise, since this doesn't change much from what we shipped in the beta, keeps the fundamental feature in place, and still provides a suggestion in the IDE so people can know something's up. 👍
"The `bundledPackageName` option must be provided when using outFile and node module resolution with declaration emit.": { | ||
"category": "Error", | ||
"The `bundledPackageName` option should be provided when using outFile and node module resolution with declaration emit.": { | ||
"category": "Suggestion", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is currently not desirable i think... because we do not distinguish between suggestion or error and build will fail with corresponding exit code instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's fair, we do kinda have a separate suggestionDiagnostics
list in the checker.
I don't think we have one in program
yet, but we'd need to add one and concatenate it with the checker's suggestions. (Inside getSuggestionDiagnostics
in program.ts
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but that means it would reported only in editor ... and not in build i am not sure thats desirable either... program construction messages should be consistent in my opinion..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, suggestions are only actionable in the editor at present, no? We don't have a --showSuggestions
compiler flag or anything to print them on the command line at present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently suggestions are errors under noUnusedLocals like flags as far as i know so there is way to get those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diagnostics.await_has_no_effect_on_the_type_of_this_expression
is unconditionally a suggestion (see its usage in checkAwaitExpression
), as is Diagnostics.Numeric_literals_with_absolute_values_equal_to_2_53_or_greater_are_too_large_to_be_represented_accurately_as_integers
in checkNumericLiteralValueSize
, so we do already have suggestions you can never see on the command line by conversion into an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And these still have value because we use them to trigger quickfixes!)
Unfortunately, we never finished reviewing this PR. It is pretty old now, so I'm going to close it to reduce the number of open PRs. |
Alternative to #41499 where the compiler flag instead recommends using
bundlePackageName
on theout
in your JSON instead of outright removing the feature.