microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.74k stars 12.46k forks source link

Consider deprecating namespaces declared with the `module` keyword #51825

Open DanielRosenwasser opened 1 year ago

DanielRosenwasser commented 1 year ago

Background

Today, TypeScript has two uses of the module keyword.

The first is called an "ambient module declaration".

declare module "some-library" {
  // ...
}

declare module "*.css";

While we often prefer people author declaration files and submit them to DefinitelyTyped, the declare module syntax here is fine.

The second use is for namespace declarations.

module foo {}
declare module bar.baz {}

This is odd - these are called "namespace" declarations? Well, namespace declarations support using either the namespace keyword, or the module keyword. You could replace module with namespace in that last code snippet and it would work the same.

But why? Why support two ways to do the same thing? Because originally, TypeScript was designed with speculation that JavaScript would have two organizational schemes: "external modules" and "internal modules". External modules were files with isolated top-level scopes that could import/export values. Internal modules were named blocks of code that could export values.

Eventually, JavaScript just got "external modules" and called them "modules". TypeScript was left with two concepts and eventually it was too confusing. So in TypeScript 1.5, we introduced the namespace keyword to make the distinction clearer. Over time, the community preferred using the namespace keyword and linters discouraged the use of module in their place.

Recently, TC39 has been discussing two additions to JavaScript - module expressions and module declarations. These proposals have some overlap with namespaces, but ultimately act different and enable different functionality. Regardless of whether we support the proposals or they go in, during discussion, most people were surprised that the use of module for namespaces wasn't at least deprecated.

As we discuss TypeScript 5.0, we're considering some cleanup in the flag and language space.

Deprecation Proposal

In https://github.com/microsoft/TypeScript/issues/51813, we discussed the matter and felt comfortable with deprecating the use of the module keyword in namespace declarations. Deprecation plans are guided by plans discussed at https://github.com/microsoft/TypeScript/issues/51000.

What this would amount to is a graceful parse on any namespace declaration declared with module. In editor scenarios, our language service would provide a quick fix to replace that keyword with namespace. When "ignoreDeprecations"": "5.0" is turned on, we would still provide an editor suggestion which would enable the same quick fix.

Caveats

This plan seems sound in implementation files; however, if a module-declared namespace appears in a declaration file from a 3rd party package, you're out of luck. At best, we could only error on implementation files.

So it's unclear if we'll be stuck with module keyword-declared namespaces in .d.ts files forever, or if this is possibly the first step in phasing that out too. Even if we wanted to support the module declarations proposal, it seems unclear whether we could when declaration files still contain module keyword-declared namespaces.

Reconsideration

Here's a few points to consider:

fatcerberus commented 1 year ago

To some extent, erroring on a keyword that no new code uses anyway feels like a break for the sake of purity. It heavily favors conceptual cleanliness for implementers without much benefit for people using TypeScript.

If module as an alias for namespace isn’t deprecated, won’t it eventually clash with the mentioned ES proposal(s)? So it seems like maintaining the status quo simply isn’t an option in the long term, and the points made under “Reconsideration” are therefore moot. See: the turmoil caused by experimentalDecorators (which is flagged!)

robpalme commented 1 year ago

Thank you for raising this!

This looks like an excellent deprecation opportunity that will reduce the current confusion caused by having two ways to declare namespaces. Especially when it's clear cut that one keyword is preferred (namespace) and the other keyword is legacy (module). It's great that TypeScript can evolve and steer users towards intended usage.

Experience in Bloomberg

Here is some context on general namespace usage within the Bloomberg JS environment. This runtime approximates a pure ES modules-based environment which means no need to cater to CommonJS.

Linting in Bloomberg’s environment ensures that namespaces are only permitted in declaration files and can only use the modern declare namespace syntax. Meaning namespaces are not used in any files that generate JavaScript. This ensures that non-JS syntax is only used for static type annotations that disappear at build-time.

When one imposes linting restrictions on a developer community, it usually results in questions like "why am I not allowed to do that?" The internal feedback on these restrictions we've heard is:

The General Case for Removal

(Let's ignore the specifics of what module might be used for in future)

The main argument I would make here is that it's positive for the TypeScript ecosystem as a whole to minimize confusing relics. And it's positive for the JavaScript ecosystem to gain access to prime ergonomic syntax.

Given the modern-day lack of enthusiasm for any kind of namespace usage, module-keyword namespaces are a niche of a niche. Whilst they may have a meaningful count on public code searches, they are not widely understood. Even experienced TypeScript developers struggle to recall and explain the difference between ambient module declarations and module-keyword namespaces. Fixing this confusion via a clear and decisive deprecate-then-remove story is a net win that justifies the migration cost in my opinion.

With any migration, it's all made easier by starting earlier, which means starting as soon as possible on the implementation files. With the current deprecation proposal, that will be around three years until full removal from implementation files. That sets the scene for the subsequent removal from declaration files once usage is low enough.

Next Step Suggestions

Auto-Modernizing Declaration Emit

Today compiling the TypeScript file declare module foo {} results in the DTS file declare module foo {} rather than the preferred DTS file declare namespace foo {}

To accelerate migration within the declaration file ecosystem, maybe we could immediately update TypeScript's declaration emitter to output modern syntax even if the implementation file continues to use the legacy syntax.

DefinitelyTyped Migrations

This could be a big topic all by itself. Given that there is some level of ecosystem centralization around DefinitelyTyped, can we seed trends there to assist declaration syntax migrations?

I'd like to hear more from DT experts on this.

Telemetry

Another big topic. VS Code already implements opt-out remote reporting of TypeScript language server crash stacks today. The documentation says it also passes on some usage data. Maybe this could be expanded, in a very minimal anonymized way, to convey when usage of a legacy TypeScript feature has been detected. This would help provide confidence that the final removal from declaration files could proceed without impacting too many users.

Summary

Deprecating and removing the legacy keyword usage seems like an excellent move and is highly appreciated. Whilst the keyword could be left around forever as a curiosity, taking proactive steps towards improving both the TypeScript and JavaScript languages helps all users and is the kind of healthy stewardship TypeScript is known for.

robpalme commented 1 year ago

@CC972 is about to start work on Auto-Modernizing Declaration Emit as described above supported by @dragomirtitian.

JoshuaKGoldberg commented 1 year ago

DefinitelyTyped Migrations

I'm not a DT expert but have been working with DT folks recently on https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/65993. The points you've suggested all sound very doable to me.

Can we enhance the DefinitelyTyped linting to warn/reject on deprecated syntax in declaration files?

Yes: https://typescript-eslint.io/rules/prefer-namespace-keyword. The repo already has support for typescript-eslint rules. We can enable that rule in the repo and use the rule's auto-fixer.

Can we automatically generate PRs to the source repos that contribute to DefinitelyTyped?

Yes, and I'd be happy to help with that. One can imagine a script like:

for repo in a b c; do
  gh repo clone a
  cd a
  npx eslint . --config /path/to/just/prefer-namespace-keyword/config --fix
  # git commands to create branch and send PR
  cd ..
end
robpalme commented 1 year ago

My goodness. @JoshuaKGoldberg I reviewed your linked DT issue and you are not messing around.

This is an excellent plan. Thank you for volunteering.

If you need anything, please shout and I will arrange assistance.

JoshuaKGoldberg commented 1 year ago

I started on https://github.com/DefinitelyTyped/DefinitelyTyped/pull/66292. It was actually pretty quick. If you want me to apply the same fixing to any other repo, I can be shouted at here/anywhere and/or tagged an issue. (@robpalme I do love the idea of giving you an excuse to tell me to go do work 😄)

The work to deprecate is higher than the maintenance of the module keyword over the last 8 years. In fact, writing this deprecation plan was more work than that.

While the maintenance cost may be near-zero, the education cost is likely higher. I've found that I've had to repeatedly explain to learners why searches for "typescript modules" & similar terms end up with this old module keyword. It's a mild pain that sometimes trips people up. +1 to removing on those grounds.

robpalme commented 1 year ago

You are a good egg, Josh. Thanks for pushing this forwards.

robpalme commented 7 months ago

As a refresher on the current status:

robpalme commented 7 months ago

After speaking with @RyanCavanaugh it seems like there may still be some concern about impacting active projects that want to upgrade TypeScript but also consume legacy DTS files. For example, where they vendored/snapshotted DTS files containing the old keyword from an unmaintained project.

So what can we do now to keep things moving forwards?

JSDoc offers the /**@deprecated*/ tag that does not error, but provides visual feedback of the deprecation along with potential for a quickfix.

image

How about we do the same for the module keyword? This would only be at the declaration site and only on the keyword, not the identifier.

module

A built-in quickfix would be offered to convert this to namespace. This would provide a useful nudge forwards to the modern keyword without breaking compatibility.

DanielRosenwasser commented 7 months ago

I think we'd be okay with something that issues a language service deprecation diagnostic. I don't know if we really need the quick fix, but not opposed either.

RyanCavanaugh commented 7 months ago

57913