Allow cross-project references to const enums in isolatedModules when referenced project has preserveConstEnums

[andrewbranch]

Partial fix for #37774
Fixes #57833

[jakebailey]

With this change, does TS itself compile with isolatedModules enabled? I think this may have been one of (or the only?) blocker for enabling it...

[andrewbranch]

There are a few fixable failures, but yes.

[timocov]

Allow cross-project references to const enums in isolatedModules when referenced project has preserveConstEnums

Fixes #37774

Does it work in the build mode only (with project references) or it is trying to detect closest tsconfig for a given d.ts file? If it works for project references only, then it might not fully fix #37774.

[andrewbranch]

It only works with project references, so yeah, it’s a partial fix for #37774.

[jakebailey]

Will this break the case where someone can run -p on a project and it work? It seems like this will end up breaking without build mode because the references won't exist.

[andrewbranch]

Yeah, you’re right. If we want to avoid that inconsistency, we have to implement an emit change as discussed in #37774. Alternatively, we could just delete this error altogether and leave users to reference const enums at their own risk.

[sheetalkamat]

Whether run --build or --p if you have setup project references in config the result should be same.

[jakebailey]

I hate to make more work, but can we have a test to verify this? Would be helpful given I don't think this is a good idea if it doesn't work. Otherwise, this would be great and would enable a very similar fix for #51530.

[andrewbranch]

I didn’t notice this, but that is already what the unnittests/tsc/projectReferences.ts case is testing.

[jakebailey]

Oh dang, you're right. Nice.

[jakebailey]

In the process of attempting to build #51530, I realized that getRedirectReferenceForResolutionFromSourceOfProject doesn't give you back "yourself", so you'll still get the error for something defined locally. Is that intended?

Maybe that's ok for this particular case, I'm just having trouble getting a version of this check that's "another project has the flag enabled and redirect or this project has it enabled and it's not just some random file but one that will be emitted)...

[andrewbranch]

I didn’t think about that case, but I think it’s ok. Including your project’s outputs as your project’s inputs is a misconfiguration in all cases, I think.

[jakebailey]

Including your project’s outputs as your project’s inputs is a misconfiguration in all cases, I think.

I didn't really mean including the outputs, but rather that there's no redirect for a local project by definition, and therefore the check comes out to "false" (when I am personally looking for a way to make it true for the local project files too).

[andrewbranch]

I guess I don’t understand the scenario you’re saying then—if you reference a const enum in your own project, it won’t be ambient since you’ll include the input file, and would never get this error. If you’re referencing another project, you’re likely to be referencing an output file, which makes the declaration ambient.

[jakebailey]

So the part where it won't be ambient answers my question in the sense of "ok for this particular case".

So, past that, I'm just looking ahead to #51530 where I'm trying to not error on uses of const enums when preserveConstEnums is true, but only if we know the value is actually going to exist (i.e. is provided by a file in the current project or referenced project where preserveConstEnums would have emitted the runtime enum).

[andrewbranch]

I think the same ambient/non-ambient breakdown will apply there.

[jakebailey]

Anyway that doesn't block anything; I assumed Sheetal's approval was enough but here's another one 😄

[jakebailey]

I will send a WIP PR showing what I mean for the above thing later.