Add flag to allow access to UMD globals from modules#30776
Add flag to allow access to UMD globals from modules#30776andrewbranch merged 8 commits intomicrosoft:masterfrom
Conversation
|
Need to revert submodule updates, otherwise LGTM |
|
This flag quite literally just suppresses the error since we already "do the right thing" in the presence of the error with respect to types. Should we not just concede to community demand and add a generalized |
|
@weswigham good point. I'm sure I'm missing some prior discussion here, but aren't there some error codes that are virtually unsuppressible, or would have horrifying consequences if suppressed? This particular error is a great example of something that would work, but I'm thinking about e.g. parser errors... suppressing a diagnostic from I guess a better summary of what I'm asking is—while I don't personally feel the need to try to force users into writing better TypeScript against their will, would a feature like that, on average, actually result in a better user experience? Or would it inadvertently wreak more havoc than solve problems? As a newbie, this is a legitimate question, not rhetorical, and I'd happily defer to the team's consensus. |
We already have |
IIRC that's not the case. It doesn't suppress parse errors and declaration emit diagnostics (don't know about binder diagnostics) |
We can exclude those from the errors we'll suppress, too, then. I'm just thinking that adding a |
andrewbranch
force-pushed
the
feature/10178
branch
from
1647187 to
786753d
Compare
| const merged = getMergedSymbol(result); | ||
| if (length(merged.declarations) && every(merged.declarations, d => isNamespaceExportDeclaration(d) || isSourceFile(d) && !!d.symbol.globalExports)) { | ||
| error(errorLocation!, Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead, unescapeLeadingUnderscores(name)); // TODO: GH#18217 | ||
| errorOrSuggestion(!compilerOptions.allowUmdGlobalAccess, errorLocation!, Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead, unescapeLeadingUnderscores(name)); |
There was a problem hiding this comment.
Should this diagnostic change to additionally mention or enable allowUmdGlobalAccess if?
There was a problem hiding this comment.
Since this is errorOrSuggestion based on the compiler option, we would have to use two different messages depending on the value of the option (since users with it enabled should not see a message that says "Enable the 'allowUmdGlobalAccess' compiler option")... do you think that's worth it?
There was a problem hiding this comment.
I'd leave as-is. In general for errors we think are usually correct, we try not to mention commandline flags to disable the error, since people will often try disabling the error before even checking if their code works or not
| export as namespace Foo; | ||
|
|
||
| // @filename: a.ts | ||
| /// <reference path="foo.d.ts" /> |
There was a problem hiding this comment.
Is this a requirement in this file?
Given the original issue #1078 is sooo convoluted, it may be well worth a bit of write-up how this new feature works.
P.S. I have a feeling it may be extremely useful in some scenarios I am involved, but need a more defined behaviour/usage/spec to know for sure.
There was a problem hiding this comment.
You're right, the original issue is a lot to absorb, but this “feature” is actually quite simple. All it does is suppress a particular error message. This one:
The triple slash reference isn't related to this feature at all; it's related to the @noImplicitReferences option which is just a configuration for our test harness.
Does that clear things up, or do you have any additional specific questions?
There was a problem hiding this comment.
Thanks, all clear.
It's awesome: from complex to trivial in a couple sentences.
8 participants
Closes #10178.
A summary, since the issue discussion is quite long: the
allowUmdGlobalAccessflag will allow accessing a global defined in a UMD module from anywhere, including from modules. This could potentially be unsafe since not all UMD modules define a global in the presence of a module system, but under some circumstances you really might need the ability. E.g. in a browser environment where one UMD module loads before require.js, that module will only be available as a global. So, this flag is an escape hatch for users stuck in scenarios like that.