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

Change keyword «with» #3

Closed
inoyakaigor opened this issue Oct 23, 2019 · 61 comments
Closed

Change keyword «with» #3

inoyakaigor opened this issue Oct 23, 2019 · 61 comments

Comments

@inoyakaigor
Copy link
Contributor

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

@inoyakaigor inoyakaigor changed the title Change keyword with Change keyword «with» Oct 23, 2019
@chicoxyzzy
Copy link
Member

chicoxyzzy commented Oct 23, 2019

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 }

@xtuc
Copy link
Member

xtuc commented Oct 24, 2019

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?

@littledan
Copy link
Member

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 ....

@devsnek
Copy link
Member

devsnek commented Oct 29, 2019

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
Copy link
Member

@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.

@justinfagnani
Copy link

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
Copy link
Member

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
Copy link
Member

chicoxyzzy commented Nov 1, 2019

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
Copy link
Member

devsnek commented Nov 2, 2019

the exact grammar should be entirely primitive literals

@chicoxyzzy
Copy link
Member

so

import 'module' with something: Symbol();

should be a valid grammar too?

@devsnek
Copy link
Member

devsnek commented Nov 2, 2019

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

@chicoxyzzy
Copy link
Member

chicoxyzzy commented Nov 2, 2019

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
Copy link
Member

chicoxyzzy commented Nov 2, 2019

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
Copy link
Member

chicoxyzzy commented Nov 2, 2019

Maybe it's even better to use record syntax there

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

@littledan
Copy link
Member

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.

@devsnek
Copy link
Member

devsnek commented Nov 6, 2019

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

@chicoxyzzy
Copy link
Member

chicoxyzzy commented Nov 7, 2019

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.

@gibson042
Copy link

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").

@Janpot
Copy link

Janpot commented Nov 8, 2019

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
Copy link

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

Yeah, let's go with eval 😉

@ForsakenHarmony
Copy link

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"™

@ChoqueCastroLD
Copy link

ChoqueCastroLD commented Dec 11, 2019

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
Copy link

xlaywan commented Dec 11, 2019

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}

@justinfagnani
Copy link

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

@ChoqueCastroLD
Copy link

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

xtuc added a commit that referenced this issue Jun 16, 2020
xtuc added a commit that referenced this issue Jun 16, 2020
@matthewp
Copy link

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.

@saschanaz
Copy link

#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
Copy link
Member

littledan commented Jun 24, 2020

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

@saschanaz
Copy link

saschanaz commented Jun 27, 2020

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
Copy link
Member

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

@Andrew-Cottrell
Copy link

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.

@xtuc xtuc removed this from the stage 3 milestone Jul 10, 2020
@Andrew-Cottrell
Copy link

Andrew-Cottrell commented Jul 10, 2020

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;

@noinkling
Copy link

noinkling commented Jul 12, 2020

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
Copy link

Andrew-Cottrell commented Jul 13, 2020

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
Copy link

noinkling commented Jul 14, 2020

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
Copy link

Andrew-Cottrell commented Jul 14, 2020

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
Copy link

Andrew-Cottrell commented Jul 14, 2020

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.

@xtuc
Copy link
Member

xtuc commented Jul 17, 2020

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).

@xtuc
Copy link
Member

xtuc commented Jul 21, 2020

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

@Alhadis
Copy link

Alhadis commented Sep 16, 2020

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
Copy link
Member

ljharb commented Sep 17, 2020

To me, while implies a condition that will change.

@Alhadis
Copy link

Alhadis commented Sep 18, 2020

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

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

@ljharb
Copy link
Member

ljharb commented Sep 18, 2020

assert is shorter :-p

@Alhadis
Copy link

Alhadis commented Sep 18, 2020

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
Copy link
Member

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

@dtinth
Copy link

dtinth commented Jul 28, 2021

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)

@nicolo-ribaudo
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests