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

Use object-literal-like notation for attributes? #5

Closed
justinfagnani opened this issue Nov 1, 2019 · 16 comments · Fixed by #70
Closed

Use object-literal-like notation for attributes? #5

justinfagnani opened this issue Nov 1, 2019 · 16 comments · Fixed by #70
Milestone

Comments

@justinfagnani
Copy link

To me, reading the key-value structure of attributes is a little difficult without the common structure of braces. An option is to use object-liter-like notation:

import {foo} from 'module' with {type: 'json'};

Since braces are already used with imports, and they aren't actual expressions, I don't think this would be any more confusing than what we have now. Both uses may look a bit like expressions (destructuring and object-literals respectively) and that's nice to intuitively understand some of the syntax, but they aren't and this is ok because import is special.

@littledan
Copy link
Member

Personally, I'd like to minimize the number of analogous cases we introduce. I think each one adds a bit of confusion. So, I'd prefer to omit the braces.

@chicoxyzzy
Copy link
Member

chicoxyzzy commented Nov 7, 2019

I'm in favor of this. I agree with @justinfagnani on readability. Besides that (as I mentioned in #3), this look pretty similar to dynamic import variant and if attributes line is very long, than multiline attributes should be considered.They look better with curly braces (this also makes trailing comma possible).

@bmeck
Copy link
Member

bmeck commented Nov 8, 2019

I think as long as the attributes are able to be wrapped in {} to become an Object literal it seems a fair tradeoff to allow some copy pasting.

@littledan
Copy link
Member

@bmeck I am trying to understand; does this mean you would be OK both with or without braces?

@bmeck
Copy link
Member

bmeck commented Nov 8, 2019

I think the argument for wanting braces/adding familiarity of syntax is less compelling if the non-brace syntax still allows copy pasting albeit requiring a wrapping {/*paste here*/}. I'm fine on either, just pointing out that the usability concern of braces enhancing copy/paste between locations seems somewhat weak.

@littledan
Copy link
Member

Thanks for clarifying. I think this matches my intuition as well.

@jridgewell
Copy link
Member

There's an ASI hazard without braces:

import { foo } from 'foo.js' with type: "json"
  foobar: "baz";

labels (while rare) can't be differentiated. Is it supposed to be { type: "json", foobar: "baz"} or { type: "json" }; foobar: "baz" (a labeled string statement).

If we consider this to be arbitrary metadata proposal, it becomes more likely that we'll have multi-line metadata, making this more likely to hit.

@devsnek
Copy link
Member

devsnek commented Dec 7, 2019

there'd likely be a comma there:

import { foo } from 'foo.js' with type: "json",
  foobar: "baz";

@jridgewell
Copy link
Member

Ahhh, you’re right.

@littledan littledan added this to the stage 3 milestone May 20, 2020
@littledan
Copy link
Member

Personally, I still prefer the no-brackets notation, but I'm open to using the brackets here.

I'd like to propose that, while the choice of a key-value notation is pretty core for Stage 2, the question of whether or not we use brackets is a more detailed/superficial decision, which we could continue debating and conclude for Stage 3. Do folks agree?

@gibson042
Copy link

In addition to having nice symmetry for import expressions, I believe braces or other delimiters are actually required to preserve future potential for solving things like #56 (i.e., the trailing comma case mentioned above). The no-braces syntax cuts off too much IMO.

@littledan
Copy link
Member

@gibson042 I guess I'm still not convinced that we shouldn't solve that within the key/value-only syntax. Nevertheless, I'm not extremely against brackets, it's more of a minor preference.

Do you think we need to resolve this brackets-or-no-brackets question before Stage 2, or can we continue discussing it between Stage 2 and 3?

@gibson042
Copy link

gibson042 commented Jun 1, 2020

I don't think the question of braces vs. no braces should block Stage 2.

@xtuc
Copy link
Member

xtuc commented Jun 13, 2020

I don't have a preference for braces vs no braces, but I'm not convinced that braces would address #56.

In general, my understanding is that braces seems to allow better UX because of copy pasting, traling comma or being closer to the dynamic import. My only concern is that the static restriction on value will confuse developers that think it's just an object.

@chicoxyzzy
Copy link
Member

No-braces syntax could be even more confusing. Also we already use curly braces in imports and they are not an object destructuring.

xtuc added a commit that referenced this issue Jun 15, 2020
xtuc added a commit that referenced this issue Jun 15, 2020
xtuc added a commit that referenced this issue Jun 15, 2020
@xtuc
Copy link
Member

xtuc commented Jun 15, 2020

I think we came to a conclusion to add braces. I sent a PR and once it's merged we will close this issue. Thanks

xtuc added a commit that referenced this issue Jun 15, 2020
@xtuc xtuc closed this as completed in #70 Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants