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

Add a rule to the compiler options to disallow the assert keyword for import attributes #58453

Open
6 tasks done
petamoriken opened this issue May 7, 2024 · 10 comments
Open
6 tasks done
Labels
Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@petamoriken
Copy link
Contributor

petamoriken commented May 7, 2024

πŸ” Search Terms

"import assertions", "import attributes", "assert"

βœ… Viability Checklist

⭐ Suggestion

Currently Stage 3 Import Attributes deprecates the assert keywords and is working to remove them from the spec if possible. V8 will remove support for the assert keywords, so Node.js and Chrome will follow suit.

Hey, quick update on this. Node.js is planning to remove support for assert in v22 (which will be released in April) and Chrome in v126 (which will be released in May).

denoland/deno#17944 (comment)

I would like to remove support for the assert keywords from TypeScript as well, and would like to add disallowAssertKeywords to the compiler options as a beginning.

FYI

JSR bans publishing of TypeScript/JavaScript files that use the assert keywords.
jsr-io/jsr#427

I've made a PR for swc, but it is pending because I can't include features that are not in tsc.
swc-project/swc#8913

πŸ“ƒ Motivating Example

The following code will fail if the disallowAssertKeywords option is enabled.

import foo from "./foo.json" assert { type: "json" };

πŸ’» Use Cases

  1. What do you want to use this for?

Deno / JSR

  1. What shortcomings exist with current approaches?

Deprecated the assert keywords cannot be rejected

  1. What workarounds are you using in the meantime?

N/A

@RyanCavanaugh
Copy link
Member

I think the most likely course of action is to deprecate the syntax in 6.0 with a compat flag for a little bit. A lint rule is probably the best choice if you need to ban this right away in some other scenario

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels May 8, 2024
@petamoriken
Copy link
Contributor Author

I think the most likely course of action is to deprecate the syntax in 6.0 with a compat flag for a little bit.

πŸ‘

The DENO_FUTURE=1 environment variable disables the assert keywords in JavaScript, so I would like to gradually disable assert keywords in TypeScript as well by adding the disallowAssertKeywords option.
denoland/deno#23541

A lint rule is probably the best choice if you need to ban this right away in some other scenario

The no-import-assertions rule is already implemented in deno_lint and is enabled by default.
denoland/deno_lint#1209

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.6.0 milestone May 10, 2024
@RyanCavanaugh RyanCavanaugh added Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels May 10, 2024
@a-tarasyuk
Copy link
Contributor

@RyanCavanaugh Should the disallowAssertKeywords option be added, or would this be considered an addition to the list of deprecated items?

@MartinJohns
Copy link
Contributor

disallowAssertKeywords sounds super generic and can mistakenly interpreted to refer to asserts too. And "keywords" is pluralized, but I'm not sure to what other keyword this option refers to?

Perhaps something like disallowAssertImports would be better?

@a-tarasyuk
Copy link
Contributor

The option name is open to discussion. However, it's more important to determine whether we need to add this new option or if this deprecation should be handled by the ignoreDeprecations option.

@RyanCavanaugh
Copy link
Member

Sorry for the confusing metadata on this one. We think we'll just hold off on erroring until 6.0, at which point it'll become parsed-with-an-error, then just quit parsing at 6.5

@robpalme
Copy link

Thanks for the update. It's great that TS can evolve by retiring syntax as well as retiring flags.

Given that this leaves the hazard of TS emitting non-ideal (maybe even non-standard/non-executable) JS without any user guidance for the next 18 months, how would you feel about landing some mitigations earlier than 6.0? For example:

  • Soft deprecation: assert does not error but does show a red squiggly + quick fix in the IDE
  • Silent upgrade: Source TS using assert is emitted as with in the JS

@RyanCavanaugh
Copy link
Member

I'd need to understand how someone would be unintentionally writing assert in an import statement to prioritize those mitigations

@robpalme
Copy link

Today Node LTS supports with and Node Latest does not support assert. In three weeks, Chrome will also not support assert.

  • assert arrived in Chrome 92 (2021-07-20) & Node 17.5 (2022-02-10)
  • with arrived in Chrome 123 (2024-03-19) & Node v18.20 (2024-03-06)
  • assert was removed in Chrome 126 (2024-06-05) & Node 22.0 (2024-04-24)

So I think it's safe to say that anyone still using assert in the TS 5.6 time-frame (Sept 2024?) is doing so unintentionally. Either due to carry-over from existing code, or someone is following out of date documentation.

Both cases would benefit from TS guidance towards with. TC39 communications on this migration has limited reach, whereas TS can surface the forwards path readily.

@petamoriken
Copy link
Contributor Author

In the TC39 meeting today we finally agreed to remove assert from the import attributes proposal.

https://x.com/nicoloribaudo/status/1818046677586153959

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript
Projects
None yet
6 participants