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

Import Attributes (Stage 3) #53656

Closed
5 tasks done
mkubilayk opened this issue Apr 4, 2023 · 16 comments Β· Fixed by #54242
Closed
5 tasks done

Import Attributes (Stage 3) #53656

mkubilayk opened this issue Apr 4, 2023 · 16 comments Β· Fixed by #54242
Assignees
Labels
Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@mkubilayk
Copy link
Contributor

mkubilayk commented Apr 4, 2023

Suggestion

πŸ” Search Terms

import attributes, import assertions, esnext

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • 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.

⭐ Suggestion

https://github.com/tc39/proposal-import-attributes

In January 2023, Import Assertions proposal was demoted back to Stage 2 to investigate a solution for the web platform's needs. In March 2023, the proposal was renamed to Import Attributes and moved back to Stage 3 (with conditional consensus - pending spec text review).

TypeScript should support the new syntax. The most relevant change for TypeScript is that the keyword changed from assert to with.

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

import("foo.json", { with: { type: "json" } });

export { val } from './foo.js' with { type: "javascript" };

The minimal option here is just to add the with syntax. Ideally there would also be two enhancements:

  • Deprecating assert: For compatibility with existing implementations, the assert keyword is still allowed by the spec until it's safe to remove it. Chrome will explore removing it. Babel is deprecating the syntax with a plan to eventually remove it in Babel 8. TypeScript could follow something similar to the 5-release deprecation process by initially suggesting migration via warnings and quick fixes - though we recognise that the deprecation process was originally intended only for flags.
  • Auto-complete for static import/re-export declarations: Auto-complete seems to be only working with dynamic import() right now. This could be expanded to also work for attributes in static declarations.

πŸ“ƒ Motivating Example

This will be new ECMAScript syntax so motivation is to be compatible with the language. The champion group considers this to be stable, even though it just changed: this is what re-promoting to Stage 3 represents.

It sounds like this would also unblock #49055 (comment).

πŸ’» Use Cases

https://github.com/tc39/proposal-import-attributes#motivation

In addition, we have an internal use case in Bloomberg to use Import Attributes as an annotation to achieve lazy-loading by deferring module evaluation. This is an early implementation of the TC39 proposal for Deferring Module Evaluation.


This issue is raised only for awareness. At this point we are not planning to implement.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Committed The team has roadmapped this issue labels Apr 4, 2023
@RyanCavanaugh
Copy link
Member

Given the Stage 3 -> 2 regression on the original proposal, we might hold off on this until it's more like Stage 3.5

@nicolo-ribaudo
Copy link

Regarding the stability of the proposal, the reason we changed the proposal so much after Stage 3 was because we found problems integrating it with the web platform. We reached broad consensus on the current semantics, which mach what HTML folks needed from the proposal.

You might have noticed that this "back to stage 2 and again to 3" dance has been very quick (3->2 in January, and 2->3 1 in March): it's because engines were very eager to ship this proposal, and we had to solve its problems very quickly. I would expect import attributes to be available in browsers very soon.

There is only one part of the proposal around which there is uncertainty: what will happen to the assert keyword. We know that the main syntax will be with, but we don't know yet if we'll be able to completely remove assert. For this reason, while I think implementing with is completely safe and it's the best way to help the ecosystem migrate quickly, I would recommend waiting before deciding what TS should do about assert. For reference, if it will remain in the spec it will be with this disclaimer (tc39/proposal-import-attributes#133):

A conforming implementation of ECMAScript should not implement Deprecated subclauses, unless necessary for compatibility with existing applications that already run in such implementation before the deprecation of the given language feature. All of the language features and behaviours specified within Deprecated subclauses have one or more undesirable characteristics. However, their usage in existing applications currently prevents their removal from this specification. These features are not considered part of the core ECMAScript language. Programmers should not use or assume the existence of these features and behaviours when writing new ECMAScript code.

Footnotes

  1. Technically the proposal is not at Stage 3 yet, but it will be officially at Stage 3 as soon as its spec text is reviewed. We are working quickly on this, and you can track progress at https://github.com/tc39/proposal-import-attributes/issues/137 ↩

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 5, 2023

Given that import attributes actually do influence resolution, would we have to specify more semantics based on the module/moduleResolution options?

@nicolo-ribaudo
Copy link

Import attributes can influence module resolution, but the currently planned attributes (i.e., just type in Node.js/web) do not. The TypeScript implementation of import attributes I expect is to just add the with keyword as an alias to assert, without actually changing any semantics.

However, if for example one day Node.js implements an attribute that does affect module resolution, I would expect TS's moduleResolution: "node16" to automatically support it.

Additionally, the paths option could be extended in the future to allow mapping (specifier,attributes)->resolved instead of just specifier->resolved (it might not be a good idea, but the proposal allows it).

@arcanis
Copy link

arcanis commented Apr 8, 2023

Although it could be useful to consider as a follow-up to have some support for with specifiers in declare module, so the following would be possible (solving the use case behind #38638):

declare module '*' with {type: 'x-url'} {
  const url: string;
  export default url;
}

@a-tarasyuk
Copy link
Contributor

Is it required to continue to support the assertClause property?

readonly assertClause?: AssertClause;

export interface AssertClause extends Node {
readonly kind: SyntaxKind.AssertClause;
readonly parent: ImportDeclaration | ExportDeclaration
readonly elements: NodeArray<AssertEntry>;
readonly multiLine?: boolean;
}

readonly assertions?: ImportTypeAssertionContainer;

Or can the assertClause property be replaced (as a Breaking Change) by the attributes property?

export interface ImportDeclaration extends Statement {
    ...
    readonly attributes?: NodeArray<ImportAttribute>;
    // readonly attributes?: ImportAttributes; ?
}

Could someone, please clarify if this change in terminology from assertions to attributes is correct or if I have misunderstood something?

@DanielRosenwasser
Copy link
Member

I think we could possibly mark assertClause as deprecated and add a shim that redirects to attributes for some number of TypeScript versions. Similarly, AssertClause could be an deprecated type alias to ImportAttributes.

@DanielRosenwasser
Copy link
Member

(@rbuckton probably has opinions here though)

@Jarred-Sumner
Copy link

Jarred-Sumner commented May 11, 2023

We'd love to support using import attributes in Bun in both the bundler and the runtime for things like:

// return a Uint8Array
import placeholderBytes from './placeholder.png' with {encoding: "binary"};

// return a data:image/png;base64, as a string
import placeholderUrl from './placeholder.png' with {encoding: "base64url"};

// note: no idea if "encoding" would be the attribute name used here nor what the specific values for these should be, just "some way for this to work"

We have implemented parser support (through the transpiler, not in JSC) for the with keyword similarly to assert. But probably nobody will use it until other tools (TypeScript, Prettier, eslint) can parse it without reporting a syntax error. It would also be nice to be able to add types for this.

@andrewbranch
Copy link
Member

@nicolo-ribaudo can we get a clarification on the official status of the proposal? I noticed tc39/proposal-import-attributes#137 is closed with all boxes checked, but tc39/ecma262#3057 is still getting review activity.

@nicolo-ribaudo
Copy link

Remaining review items are just editorials about spec wording, the semantics are settled :) (unless there is significant implementation feedback, like for any stage 3 proposal)

@andrewbranch
Copy link
Member

Are those review items pending stage 3, or is the proposal officially stage 3 and those review items are pending stage 4 / acceptance into the final spec?

@nicolo-ribaudo
Copy link

nicolo-ribaudo commented Jul 14, 2023

Officially stage 3, the editors approved the current proposal spec

@rbuckton
Copy link
Member

I think we could possibly mark assertClause as deprecated and add a shim that redirects to attributes for some number of TypeScript versions. Similarly, AssertClause could be an deprecated type alias to ImportAttributes.

We can probably just mark it as deprecated and set it for both properties until some future time where we remove it in a major release. If we want to remove it sooner we could use Object.defineProperty to attach a getter that reports a deprecation warning.

@a-tarasyuk
Copy link
Contributor

a-tarasyuk commented Jul 23, 2023

We can probably just mark it as deprecated and set it for both properties until some future time where we remove it in a major release.

@DanielRosenwasser @rbuckton Does that mean the implementation for assert (parser/types/tests etc.) should be preserved, and with should be added as a new feature without changing the behavior of assert? Or should assert be replaced by with (with has differences in implementation), with deprecated errors (parser/checker)/API aliases/JSDoc deprecations?

@rbuckton
Copy link
Member

with has differences in implementation

Are you talking about the runtime implementation, or the TypeScript compiler implementation?

Until everyone has moved off of assert, we need to preserve how assert is parsed as well as also support with, though we should probably report an error when using the assert syntax. The AST node should continue to have the existing assertClause property, but marked as /** @deprecated */. The NodeFactory should be able to take either an assert clause or a with clause. We may even want to report a deprecation warning to the console when you use NodeFactory methods that produce or update an AssertClause, or when one is provided to a factory method, but only if we can find a way to do so for only custom transformations or API use (i.e., don't report from tsc or the language service when the compiler uses them regularly, since we'll report normal diagnostics in those cases).

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
Development

Successfully merging a pull request may close this issue.

10 participants