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

Allow isolatedModules with global scripts, and better match build tool capabilities #46295

Closed
wnayes opened this issue Oct 10, 2021 · 3 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@wnayes
Copy link
Contributor

wnayes commented Oct 10, 2021

Suggestion

The --isolatedModules flag is typically used to ensure compatibility with build tools that don't have access to type information.

One restriction currently is that global (non-module) scripts are not allowed at all when this flag is enabled. This restriction does not match up with the capabilities of tools like the Babel @babel/plugin-transform-typescript plugin. The Babel plugin is able to process global scripts and even has impartial namespace support.

It is difficult to migrate an existing codebase to use the Babel transform plugin if it has a lot of existing global script / namespace code.

⭐ Suggestion

Minimally, it would be nice if the isolatedModules option did not raise errors for global scripts. (Enforcing "all modules" seems like it could be separate option.)

Ideally, there would be a way to configure the TypeScript compiler to do type checking that matches the capabilities of build tools like the Babel plugin.

For example, the isolatedModules option could behave as follows:

  • For module files, just behave the same.
  • For global scripts, allow them and enforce that there are no unqualified namespace references.

📃 Motivating Example

The example from the Babel documentation, modified slightly:

// FileA.ts
namespace Company {
  export const V = 1;
}

// FileB.ts
namespace Company {
  export const W = V;
}

As the Babel documentation describes, TypeScript knows to emit Company.V inside the second namespace because of its understanding of types declared globally across files, but Babel has no idea and just emits V.

It is not uncommon for existing legacy codebases to have a lot of code like this. A company could have put most of their code inside a namespace and be frequently accessing types without qualifying.

💻 Use Cases

  • Teams trying to switch to the Babel transform for emit. They may be writing new code as modules, but still have a corpus of older global script code that uses namespaces. It would be nice to not have to completely rewrite and eliminate global scripts in order to (safely) use the Babel transform and the isolatedModules option.

Alternatives

It might be possible to write something that can identify problem cases like unqualified namespace references. (A one-off script, maybe even a linter rule using type info.) That could help with getting a codebase into a state where the code emits safely through Babel. It doesn't seem that this would be convenient to use while maintaining code going forward.

🔍 Search Terms

babel transform namespaces

✅ Viability Checklist

My suggestion meets these guidelines:

  • ✅ This wouldn't be a breaking change in existing TypeScript/JavaScript code
    • It would be altering isolatedModules to raise fewer errors, so possibly a noticeable change to existing behavior.
  • ✅ This wouldn't change the runtime behavior of existing JavaScript code
  • ✅ This could be implemented without emitting different JS based on the types of the expressions
  • ✅ This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • ✅ This feature would agree with the rest of TypeScript's Design Goals.
@andrewbranch
Copy link
Member

The litmus test for what isolatedModules errors on is what ts.transpileModule can handle, and it handles these locally merging namespaces just fine. I think as a matter of principle, we would say that when Babel or other transpilers incorrectly handle the locally merging case, that’s a bug on their end, though in reality I don’t think it’s one we could expect them to fix.

I think the broader suggestion of allowing scripts to be compiled under isolatedModules as long as they don‘t contain problematic code is a much more compelling suggestion. Two things we like to avoid are adding compiler flags and investing in legacy constructs like namespaces. But we do like isolatedModules, and making it incrementally more useful for everyone might be worth the effort of adding an error when people try to use unqualified namespace references. On the other hand, though, I think it would be pretty tempting to just say “global files can’t use namespaces at all under isolatedModules,” which wouldn’t solve your problem 🤔. At any rate, I think this suggestion has a better chance of survival if you reframe it as “let us compile scripts under isolatedModules,” calling out the one pattern that needs to be guarded against to make that happen.

@andrewbranch andrewbranch added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Oct 12, 2021
@wnayes wnayes changed the title Add option to require qualifying namespace accesses that need type info Allow isolatedModules with global scripts, and better match build tool capabilities Oct 13, 2021
@wnayes
Copy link
Contributor Author

wnayes commented Oct 13, 2021

Thanks @andrewbranch, I agree and have reworded the issue like you suggested.

I had originally been thinking specifically about the challenges with the Babel namespace limitations, but it really is a larger problem of getting a massive codebase to be compatible with other build tools for emit.

Having isolatedModules not throw errors for global scripts alone would be a big help. That setting could then be turned on to start enforcing restrictions on modules, without having to first eliminate every single global script that also happens to still be around.

And then it's just a question of whether namespaced global scripts can be maintainable when Babel is used for emit. It seems like with the right compiler help (preventing unqualified namespace references), maybe they could be.

@andrewbranch
Copy link
Member

This was implemented in #52203. See the linked design meeting notes above and cf5c284 for details. Thanks @wnayes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants