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

Stop using extended attributes for constructors #636

Closed
Ms2ger opened this issue Feb 4, 2019 · 17 comments · Fixed by #700
Closed

Stop using extended attributes for constructors #636

Ms2ger opened this issue Feb 4, 2019 · 17 comments · Fixed by #700

Comments

@Ms2ger
Copy link
Member

Ms2ger commented Feb 4, 2019

There has been some discussion about this in #485; I'm filing this to consider changing [Constructor] in particular, since nobody seemed to particularly object to it.

I'd like to propose the following syntax:

interface Foo {
  constructor(...);
};

Without further changes, I believe the only WebIDL-defined extended attributes that would potentially start applying are [Exposed], [SecureContext] and [Unforgeable]. It seems fine to allow [Exposed] and [SecureContext], but I would disallow [Unforgeable].

We should probably not allow constructors to be specified within mixins.

Please comment asap if you have thoughts on this.

CC @yuki3 @littledan @bzbarsky @tabatkins @domenic @annevk

@TimothyGu
Copy link
Member

It sounds a bit weird to me to see an interface whose constructors are exposed in certain situations but not others, even if the interface itself is exposed. If we don’t have a concrete use case for them yet then I’d vote for just disallowing any extended attributes right now, not that it’s too difficult to change them in the future.

@littledan
Copy link
Collaborator

We discussed multiple browser-specific use cases for extended attributes on specific constructor overloads in #485. Would it be reasonable to define what it means to include extended attributes on constructor overloads in WebIDL, even if we don't have a web spec that would use that capability at the moment?

cc @kmiller68

@annevk
Copy link
Member

annevk commented Feb 4, 2019

It seems okay to me. There might even be a use case today in clarifying the exact semantics of https://xhr.spec.whatwg.org/#interface-formdata. Namely that it doesn't have an optional argument in worker environments. (I think this is what the current prose means as well, but perhaps we should move to making such things more explicit.)

@bzbarsky
Copy link
Collaborator

bzbarsky commented Feb 4, 2019

[Exposed] and [SecureContext] are both meant to affect visibility/existence, and do so consistently at the moment. What are the proposed semantics for them for constructors? And if they're different from the normal semantics, do we actually want that?

@annevk
Copy link
Member

annevk commented Feb 4, 2019

I meant that I could see rewriting that example to have two (overloaded in a sense) constructors, one exposed in Window and one in Worker. So equivalent semantics to what they mean today.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Feb 4, 2019

What would it mean to have them "not be exposed"? The constructor is not exposed per se; it just affects the behavior of the [[Construct]] of the interface object.

@domenic
Copy link
Member

domenic commented Feb 4, 2019

I think for now disallowing all of the spec-defined extended attributes is the best approach. But, allow extended attributes in the grammar, and that way UAs can use their internal extended attributes as desired.

@yuki3
Copy link

yuki3 commented Feb 5, 2019

+1 for Timothy and Domenic's approach.

@tabatkins
Copy link
Contributor

I'd be very happy to see constructors become normal methods. While I lean slightly towards calling them by the class's name, as that's how you actually call them, I'm sympathetic to the "just do it like Javascript" naming method (tho that was, I presume, at least partially motivated by the existence of anonymous classes, which WebIDL doesn't have).

@yuki3
Copy link

yuki3 commented Mar 8, 2019

@Ms2ger just as a friendly ping from curiosity. I'm just wondering if there is any update. Is something blocking? Can we help something?

@Ms2ger
Copy link
Member Author

Ms2ger commented Mar 8, 2019

I got a private suggestion that it might be better to hold off on this change in favour of doing #485 all at once; I haven't fully made up my mind yet if I agree with that.

@yuki3
Copy link

yuki3 commented Mar 8, 2019

Thanks for the update. I understand the situation. :)

@domenic
Copy link
Member

domenic commented Mar 8, 2019

Hmm. I tend to think establishing a workflow for multiple incremental changes will be better.

@littledan
Copy link
Collaborator

I was the source of this suggestion. I just was thinking about how we could get everything updated and thought several incremental changes could be costly, and that it would make sense to batch them. I haven't posted about this on #485 to make this suggestion just because I have not taken the time dedicated to make a more clear and detailed proposal.

If others want to pursue a workflow for multiple independent changes, please go ahead.

@yuki3
Copy link

yuki3 commented Mar 11, 2019

I'd personally like to have this change first compared to other changes because this proposal changes not only style but also functionality (what we can represent in Web IDL), and also because people already made agreement at some level.

I'm afraid that other purely-style changes would involve more discussions (it could be hard to make agreement on a style) and would take a long time.

@marcoscaceres
Copy link
Member

Agree with #636 (comment) . If this could land first, and that was also followed by internal slots, that would be quite immediately beneficial. Other stuff in #485 sounds great, but it's mostly bikeshedding.

@littledan
Copy link
Collaborator

OK, feel free to make this change. I agree that #485 would involve more discussion. We had previously settled on not permitting more functionality in #636 (comment) , which is why it started to seem like a purely style thing, but it's still useful for multiple browsers for custom extended attributes.

Ms2ger added a commit that referenced this issue Mar 29, 2019
Ms2ger added a commit that referenced this issue Aug 6, 2019
Ms2ger added a commit that referenced this issue Aug 27, 2019
Ms2ger added a commit that referenced this issue Aug 27, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 29, 2019
… and into .webidl files. r=peterv

The one exception, apart from tests, is a place where the constructor is being
renamed, because there is no way to support that syntactically yet.  There will
be if whatwg/webidl#636 is fixed.

Differential Revision: https://phabricator.services.mozilla.com/D39792

--HG--
extra : moz-landing-system : lando
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Aug 29, 2019
… and into .webidl files. r=peterv

The one exception, apart from tests, is a place where the constructor is being
renamed, because there is no way to support that syntactically yet.  There will
be if whatwg/webidl#636 is fixed.

Differential Revision: https://phabricator.services.mozilla.com/D39792
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
… and into .webidl files. r=peterv

The one exception, apart from tests, is a place where the constructor is being
renamed, because there is no way to support that syntactically yet.  There will
be if whatwg/webidl#636 is fixed.

Differential Revision: https://phabricator.services.mozilla.com/D39792

UltraBlame original commit: fb9468d66df42cf6e09d288e53fafcd9e7bdcbd4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
… and into .webidl files. r=peterv

The one exception, apart from tests, is a place where the constructor is being
renamed, because there is no way to support that syntactically yet.  There will
be if whatwg/webidl#636 is fixed.

Differential Revision: https://phabricator.services.mozilla.com/D39792

UltraBlame original commit: fb9468d66df42cf6e09d288e53fafcd9e7bdcbd4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
… and into .webidl files. r=peterv

The one exception, apart from tests, is a place where the constructor is being
renamed, because there is no way to support that syntactically yet.  There will
be if whatwg/webidl#636 is fixed.

Differential Revision: https://phabricator.services.mozilla.com/D39792

UltraBlame original commit: fb9468d66df42cf6e09d288e53fafcd9e7bdcbd4
travisleithead added a commit to travisleithead/design-principles that referenced this issue Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

9 participants