tc39/proposal-import-attributes

Change keyword «with»

inoyakaigor opened this issue · 61 comments

inoyakaigor commented

Keyword with already exists. Maybe better define MIME type like this:
import module from 'module' as 'json'?

chicoxyzzy commented

I don't see any problems with with in this context, it looks clean. Also type is sufficient part of syntax, it shouldn't be omitted. What I think about is if another option will be introduced, for example

const foo = import('module', { type: 'json', anotherOption: 42 })

than, how should static modules syntax look like?

import foo from 'module' with type: 'json', otherOption: 42; // ?
import foo from 'module' with type: 'json', with otherOption: 42; // ?

I'd prefer something like

import foo from 'module' with { type: 'json', anotherOption: 42 }
  • 👍2
xtuc commented

I don't feel strongly about using vs with, they seem similar to me.

@inoyakaigor
The syntax

import module from 'module' as 'json'

is more concise, but doesn't allow arbitrary attributes on the ImportDeclaration. This proposal will support more features than the type in the future.

@chicoxyzzy
A json object seems more consistent because it's a known syntax. The module's attributes are static, people might use the attribute as a regular object like:

import foo from 'module' with { type: 'json', anotherOption: process.argv }

I think adding a new syntax would remove that confusion. Or is this something we would like to allow?

  • 👍1
littledan commented

An issue with as is that it's already part of import syntax, in a different position. with would be new and non-overlapping in this context. Anyway, I acknowledge that it's been pounded into everyone's head that with is bad, so maybe we should avoid it ....

  • 👎1
  • 😕2
devsnek commented

object literals as xtuc posted is not possible because you'd have to evaluate a module to know if it shouldn't be evaluated. We could use object syntax with only literals or something, but that seems confusing. I'd imagined something like

import 'x' with y: 'z', a: 'b';
littledan commented

@devsnek Yeah, that's what I'd imagine too. To me, the curly braces make me feel like you could use any expression there, which you can't.

  • 👍1
justinfagnani commented

I'm proposing that with is reused for mixin application in: https://github.com/justinfagnani/proposal-mixins

Would this be too many separate meaning?

chicoxyzzy commented

in this statement

import 'module' with type: 'json';

'json' looks like an expression too. What literal types should be allowed in with? Only strings and numbers?

chicoxyzzy commented

In the hypothetical situation with a bunch of options when the import string becomes too long curly brackets also look better IMHO

import 'module' with
  type: 'json',
  anotherOption: 6 * 7, // should that be allowed?
  ...
  lastOption: 'hello' + 'world'; // should that be allowed?

vs

import 'module' with {
  type: 'json',
  anotherOption: 42_000n, // should that be allowed?
  ...
  lastOption: `hello ${'world'}`, // should that be allowed?
}
devsnek commented

the exact grammar should be entirely primitive literals

chicoxyzzy commented

so

import 'module' with something: Symbol();

should be a valid grammar too?

devsnek commented

@chicoxyzzy that's not a primitive literal, that's a call expression.

chicoxyzzy commented

Oh, right. In that case, my only concern is that since with values are somewhat limited (could only be primitives), there is no big difference between

import 'module' with
  type: 'json',
  anotherOption: 42n,
  ...
  lastOption: 'hello world';

and

import 'module' with {
  type: 'json',
  anotherOption: 42n,
  ...
  lastOption: 'hello world',
};

Though, curly braces option looks better and could allow trailing comma.

chicoxyzzy commented

Also those limitations look very similar to Can't convert Object with a non-const value part from Tuples and Records proposal example so perhaps some logic could be reused for with part of import statement

chicoxyzzy commented

Maybe it's even better to use record syntax there

import 'module' with {|
  type: 'json',
  anotherOption: 42n,
  ...
  lastOption: 'hello world',
|};
littledan commented

Records also allow nested expressions in them, so they don't really get us any closer...

Literal strings which are not expressions in import statements is an established concept--that's what module specifiers are. They are sort of part of the bare minimum of what we need here to make the syntax work at all. So I'd suggest using literal strings and not other expression-like constructs.

  • 👍2
devsnek commented

actually this gets me wondering about import 'x#type=json'. uses URLs, doesn't get sent to the server, backwards compat, etc.

  • 😕1
chicoxyzzy commented

Records also allow nested expressions in them, so they don't really get us any closer...

Disregard my suggestion about {| ... |}. I meant that static semantics of imports and runtime semantics of records and tuples possibly can share some logic as an abstract operation, though both proposals are on very early stages to care about that.

  • 👍1
gibson042 commented

Overloading like that collides with the intended semantics of URI fragment components ("indirect identification of a secondary resource by reference to a primary resource and additional identifying information").

  • 👍1
  • 👎1
  • 👀1
Janpot commented

posted something here but probably belongs here as well for completeness?

Parsing of the attributes could be left to hosts/compilers. For instance through querystring parsing.

import json from "./foo.json" as "type=json&anotherOption=42";

// and default behavior
import json from "./foo.json" as "json";
// would be equivalent to
import json from "./foo.json" as "type=json";
stevenvachon commented

it's been pounded into everyone's head that with is bad, so maybe we should avoid it ....

Yeah, let's go with eval 😉

  • 😄6
ForsakenHarmony commented

Have to say I agree with @chicoxyzzy

The : looks a bit weird on it's own, we don't have from: './file' either

And the first part import { a, b, c } from ... already kinda looks like an object

Currently if you have multiple attributes the syntax just gets a bit hard/confusing to read import json from "./foo.json" with type: "json", thing: "bar";

It's like an implicit "object"™

  • 👍4
ChoqueCastroLD commented

Keyword should be more explicit than "with", a cleaner syntax may be "if" as we are using simple conditions to see if the module gets loaded or not:

import { data } from "./foo.json" if type: "json"
import "./bar.wasm" if type: "wasm"

Multiple attributes should use curly braces like:

import { data } from "./foo.json" if {type: "json", executable: false}
import "./bar.js" if {type: "module", anotherOption: 30}

This allows for more options to be implemented in a not so far future.

xlaywan commented

We can use when keyword and explicit object. It will be similar to this proposal(with some restrictions) https://github.com/tc39/proposal-pattern-matching

import foo from './foo.json' when {type: 'json'}
import foo from './foo.json' when {type: 'json', anotherOption: 42}
  • 👍2
justinfagnani commented

I don't think either if, or when or any word that implies the import is conditional should be used.

  • 👍7
ChoqueCastroLD commented

@justinfagnani Could you explain a little bit more the reasoning behind your comment?

justinfagnani commented

Metadata may be used by the host for any purpose. The syntax shouldn't imply that the only purpose is to make the import conditional.

  • 👍2
hax commented

when, as or any non-reserved words will add new ASI hazards:

import foo from './foo.json'
when
{type: 'json'}
  • 👍1
ljharb commented

We can add NLTH to solve that, but also, the committee has previous consensus not to let the creation of new ASI hazards block a feature (all else being equal).

hax commented

the committee has previous consensus not to let the creation of new ASI hazards block a feature (all else being equal).

I have to declare again, as a TC39 delegate, I don't agree "previous consensus", if the new ASI hazards would cause significant cost in JS ecosystem.

MylesBorins commented
littledan commented

I think it'd be pretty weird to split up an import statement this way. You'd need both of the line breaks to have this ASI hazard. I don't think we're at the point where we should prohibit all NLTH. Of course, this feature is something that we'll seek consensus on individually, but I agree with ljharb's understanding of history that we established when considering field declarations inside classes.

  • 👍1
Andrew-Cottrell commented

Rather than base type values on file extensions, it may be less surprising to use the assigned Media Types

import foo from "./foo.css" with {type: "text/css"}
import foo from "./foo.json" with {type: "application/json"}
import foo from "./foo.wasm" with {type: "application/wasm"}

The default value for type would be "application/javascript" for compatibility with existing usage.

In a HTTP context there would be obvious correspondence between the type and the response's Content-Type header.

Please ignore the above; the topic is already being discussed in #16.

  • 👍2
xtuc commented

We only mentionned the keyword for or having, and I would like to add given, for example:

import a from "b" given {type: "json"}
import a from "b" given "json"

which, as a non native english speaker, can make sense?

hax commented

I prefer for or with because they are very simple words and already js keywords so no one will spell wrong. having and given are also simple words, but they are participles which may confuse some non-english speakers.

  • 👍2
littledan commented

For Stage 2, I want to conclude that we'll use the key/value syntax. Over time, I haven't seen a better keyword than with, so I'd like to tentatively conclude that we'll use with keyword, but I'd be open to revisiting before Stage 3 if someone comes up with a keyword we can agree is better.

  • 👍4
xtuc commented

Now that attributes need to be "check-only" if would better reflects the current semantics.
But if isn't future proof because follow-up proposals might extended the semantics.

  • 👍2
ljharb commented

@xtuc unless we wanted to have if for check-only attributes, and add with or something for different semantics:

import result from 'path/to/module' if { type: 'json' } with { reviver: 'path/to/reviver/module' };
xtuc commented

To be complete, I think it would play well with #56:

import result from 'path/to/module' if { type: 'json' } fallback { alt: 'path/to/other-module' };

I think to make some visual sense the keywords would need to be ordered, which might not be a great UX. It seems also quite verbose to me.

xtuc commented

At this point, I think it makes sense to use the if keyword instead of with and I will send a PR to update the repo.

  • 🎉1
  • 😕1
  • ❤️1
matthewp commented

As @justinfagnani mentioned using if indicates that the import is conditional, which it is not. I like @xtuc's suggestion of given here, but other similar synonyms can also work well.

  • 👍1
saschanaz commented

#72 says it is indeed conditional. Is the behavior like that import foo from "./abc.json" if { type: "json" } is no-op when it's not JSON?

littledan commented

When it's not JSON, the module graph fails to load. The conditionality is about whether it should all fail or not.

  • 👍1
  • 👀1
saschanaz commented

import ... if ... sounds like a pythonic conditional statement, IMO it would be confusing if it still runs (Edit: I mean, it still try importing and fail) when if condition doesn't match 🤔

littledan commented

When the condition does not match, the module importing it fails to load. So, it's more of an assertion.

Andrew-Cottrell commented

Another option I haven't seen mentioned — prompted by the previous comment — is assert

import result from "path/to/module" assert { type: "json" }

In contrast to if, assert may more readily be understood to imply throws-exception semantics rather than no-op semantics.

  • 👍6
Andrew-Cottrell commented

I dislike flogging a dead horse, but I want to emphasise that, to some developers, if may imply (in pseudo-code)

// import result from "path/to/module" if { type: "json" }
var meta = [from "path/to/module"];
if (meta.type === "json") {
    var result = meta.defaultExport;
} else {
    // no-op and `result` has value `undefined` when the module code is evaluated
}

While, for many developers, assert will imply a more correct interpretation

// import result from "path/to/module" assert { type: "json" }
var meta = [from "path/to/module"];
meta.type === "json" || throw new AssertionError(); // uncatchable exception and the module code is not evaluated
var result = meta.defaultExport;
  • 👍2
noinkling commented

Just adding my opinion here (for what it's worth) to say that if also feels unintuitive to me for the semantics, and reads like Ruby's/Perl's "postfix" if, or Python's ternary or comprehension syntax, and I echo the sentiment that I would not intuitively expect an exception if the condition fails. assert would be much better in my opinion.

However, if I may push back further, assert still doesn't feel very intuitive in the simple JSON import case:

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

At the end of the day, the motivation for the developer to use this feature (in this case) is because they want a file/resource interpreted by the engine/runtime as <type>, regardless of what the exact semantics are (if that weren't true, the extra syntax wouldn't be necessary in the first place). It therefore breaks expectations somewhat to have to say "ensure this file is <type>", because it doesn't inherently convey the sense (to me anyway) that I'm asking the engine to change its interpretation behaviour. Edit: I phrased that really badly/inaccurately but will leave it for posterity.

With that in mind, and noting that it doesn't seem like a good idea to compromise accurate semantics in the general case, would it be totally out of the question to make type, assuming it will be the most common application of this proposal, a special case in the interest of ergonomics? e.g:

import foo from "./foo.json" as "json" assert { ... }  // and whatever else

One of the reasons I bring this up is because I get the sense that if assert, or especially if, goes ahead, it's going to end up in a globalThis kind of situation with the wider community expressing dissatisfaction too late because it didn't enter their consciousness/they weren't made aware, especially since it was a recent change and there is a push for stage 3 scheduled in little over a week. Many (most?) people aware of this proposal still assume that as is the intended syntax (as did I until I happened upon the repo by chance today). This seems like the kind of thing that needs time for the updated proposal to disseminate and broader community input, which is why I felt compelled to speak up. I feel that at the very least the matter of ergonomics w.r.t. the keyword in the type case should be raised with the committee.

Just my 2c.

Andrew-Cottrell commented

At the end of the day, the motivation for the developer to use this feature (in this case) is because they want a file/resource interpreted by the engine/runtime as <type>, regardless of what the exact semantics are (if that weren't true, the extra syntax wouldn't be necessary in the first place). It therefore breaks expectations somewhat to have to say "ensure this file is <type>", because it doesn't inherently convey the sense (to me anyway) that I'm asking the engine to change its interpretation behaviour.

I may have misunderstood, but I thought the engine/runtime is responsible for determining how to interpret the file/resource (e.g. via file extension or Content-Type header). Previously the proposed keyword was as or with in the expectation module attributes might also be used with non-assertion semantics. The focus of this proposal changed — I believe on or before 3 June — to only support assertion semantics. However, specifically for JSON resources the import condition (neé module attribute) was always intended as a security feature to assert a requirement. If the requirement for a JSON resource is not asserted then it need not be mentioned at all. From this, possibly mistaken, point of view as "json" would not be useful to have in addition to assert or if.

// hope for the best, but may be insecure if the resource has been replaced with JavaScript
import foo from "./foo"; // unsafely import a JSON resource

// hope for the best, but module graph may fail to load if the resource has been replaced with JavaScript
import foo from "./foo" assert { type: "json" }; // safely import a JSON resource

Please correct me if any aspect of my understanding is wrong or misleadingly incomplete. I'm currently unclear if it is possible to unsafely import a JSON resource (e.g. with Content-Type: application/json) without specifying an import condition; is there an implicit/default import condition equivalent to type: "javascript"? I guess this might be host-dependent, where local runtime engines could choose to allow unsafe imports, and where web user-agents could require an explicit import condition except for JavaScript resources.

noinkling commented

I'll try to explain better where I'm coming from. Here's a quote from the readme:

Each JavaScript host is expected to provide a secondary way of checking whether a module is a JSON module. For example, on the Web, the MIME type would be checked to be a JSON MIME type. In "local" desktop/server/embedded environments, the file extension may be checked (possibly after symlinks are followed). The type: "json" is indicated at the import site, rather than only through that other mechanism in order to prevent the privilege escalation issue noted in the opening section.

Nevertheless, the interpretation of module loads with no conditions remains host/implementation-defined, so it is valid to implement JSON modules without requiring if { type: "json" }. It's just that if { type: "json" } must be supported everywhere. For example, it will be up to Node.js, not TC39, to decide whether import conditions are required or optional for JSON modules.

From my dumb, simplified, practical point of view this means:

  • Without extra syntax = JSON imports maybe/probably won't work.
  • With extra syntax = JSON imports will work.

Even though unsimplified it's more like (forgive me if I get something wrong):

  • Without proposal syntax:
    • Without JSON media type/file extension: JSON imports won't work (not as JSON at the very least).
    • With JSON media type/file extension: JSON imports won't work on web, maybe in other environments.
  • With proposal syntax:
    • Without JSON media type/file extension: JSON imports won't work.
    • With JSON media type/file extension: JSON imports will work.

This reveals that "extra syntax = JSON imports will work" isn't necessarily true, but my focus is not on the exceptional case - indeed I would expect that if there is some discrepancy with the source type (or any other easily detectable security vulnerability) an exception would be thrown; the keyword is almost irrelevant. One way I could maybe phrase this to get the point across is that the naming of assert (or if) derives from a relatively unimportant implementation detail of my (simpler, more practical) mental model.

Consider also that a dev is (generally) not going to be concerned if they included the extra syntax redundantly because the host/runtime/implementation (whatever the terminology is) didn't require it, but they will obviously take note if they don't include the syntax and it doesn't work. In that situation, how can you help but associate capability with the syntax?

In essence, the assert keyword is technically/semantically appropriate, but derives from making a strong distinction between host/implementation and the language proper. There is good reason to follow that principle, of course, but for a feature like this where the result ultimately relies on host/implementation, I argue that there may be worthwhile advantage in adopting a more "holistic" keyword specifically for the type case, because assert and if feel disjointed in relation to how I believe people are going to most commonly think about and use this feature. as (or straight up type) as a special case is intuitive and reads nicely, and doesn't oppose the check semantics, just obscures them as an implementation/spec detail.

as "json" would not be useful to have in addition to assert or if

as "<type>" would replace assert/if: { type: "<type>" } and (presumably) keep the same semantics. I put the assert part in the example to represent any other potential (non-type) assertions, just because this proposal aims to make that case generalizable, but it can also represent any follow-up proposals like with/fallback/etc.

Andrew-Cottrell commented

My apologies for misunderstanding your point, and thank you for the quote from the readme.

I argue that there may be worthwhile advantage in adopting a more "holistic" keyword specifically for the type case, because assert and if feel disjointed in relation to how I believe people are going to most commonly think about and use this feature. as (or straight up type) as a special case is intuitive and reads nicely, and doesn't oppose the check semantics, just obscures them as an implementation/spec detail.

I don't think as is suitable due to it already being used for a different purpose in import statements, but I think type could potentially be used in intuitive syntactic sugar dedicated to a common case.

One of

import foo from "./foo" type "json";
import foo from "./foo" type: "json";

could desugar to either

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

For this to work, type would be a keyword in the context of import statements. The string "json", in the above example, could be replaced with any other quoted literal string. This syntactic sugar might be allowed or disallowed when an assert/if clause is present.

One drawback is some developers might incorrectly assume this syntactic sugar would be extended for other assertions that may be specified in future proposals. Consistency with dynamic imports would also have to be considered. Complexity of the specification, documentation, runtime engines, build tools, and static analysis tools would increase.

Andrew-Cottrell commented

Although probably well known to others, it has just occurred to me import statements are similar to SQL select statements.

import {foo as getFoo}    from "./foo" if    { type: "json" };
select name as 'username' from  users  where state = 'active';

I don't know what proportion of JavaScript developers are familiar with SQL, but it might be interesting to explore the parallels, between import and select statements, for the purpose of explaining import conditions to a wider audience.

This similarity suggests where as another alternative to assert or if.

  • 👍2
xtuc commented

Thanks for your input @Andrew-Cottrell and @noinkling, we have a PR to switch the if keyword to assert: #80 . We are looking to discuss it during the next TC39 meeting (next week).

  • 👍1
xtuc commented

PR was merged, we are now using the assert keyword.

  • 👍3
  • 🎉2
  • 😕2
Alhadis commented

Has while been considered?

import foo from "source" while {type: "json"};

We already use the keyword in a similar context for do… while pairs, which is grammatically similar English:

JavaScript: do { this } while { predicate };
English:   "do (this) for as long as (predicate) remains true."

JavaScript: import foo from "source" while { assertions };
English:   "import (foo) from (source) as long as (assertions) are all correct."

assert feels... weird, because it's a verb used in an imperative voice, the same as import. This kind of implies two actions are being taken: importing and asserting. Which basically boils down to:

JavaScript: import foo from "source" assert { predicate };
English:   "import (foo) from source and assert that content
            we haven't even loaded yet satisfies (predicate)"

I'm probably just too attached to the idiomatic flow of our current syntax, which is currently well-formed English that reads quite nicely.

ljharb commented

To me, while implies a condition that will change.

  • 👍3
Alhadis commented

Yeah, I know... 😞 Stupid English doesn't have a shorter way to write expecting:

import foo from "source" expecting {type: "json"};
ljharb commented

assert is shorter :-p

  • 👍1
Alhadis commented

Also, how many things—other than content-type—will realistically need asserting? The proposal says:

Assertions in addition than type may also be introduced for purposes not yet foreseen.

The only other thing I could see being asserted is file integrity, but come on, nobody wants to see this:

import foo from "bar" assert {
	hash: "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" 
}

Moreover, assert still doesn't help if a file is syntactically valid in multiple formats. Consider:

[true]
{
	content: "Bar";
}

What happens when import("./contrived-example.css") is used? The file is legal CSS and JS (albeit useless in both), even though the author probably meant to load it as CSS. Conversely, as "css" is much clearer about the author's expectations.

I doubt this problem will crop up today (especially with the small number of formats we're concerned with), but who knows what we might be importing in another 10 years' time...

littledan commented

@Alhadis Well, the semantics are to assert that the content satisfies the predicate. This makes me glad you have this interpretation.

  • 👍1
dtinth commented

I saw a few other languages use the -ing form of verb to modify import statements, so I am surprised that the keyword “asserting” has not been brought up so far.

Examples from other languages:

  • Haskell (hiding): import Prelude hiding (zip)
  • Elm (exposing): import Post exposing (Post, estimatedReadTime)
  • 👍1
  • 👀1
nicolo-ribaudo commented

After some back and forth, and after almost settling on assert, the proposal has been updated to the following syntax:

import { x } from "foo" with { attributeKey: "attributeValue" }

Thanks everyone for taking part in the discussion!

  • 🎉2