WICG / construct-stylesheets

API for constructing CSS stylesheet objects
Other
138 stars 25 forks source link

Invalid constructor syntax #114

Open bzbarsky opened 4 years ago

bzbarsky commented 4 years ago

https://github.com/WICG/construct-stylesheets/pull/110 added a constructor operation to the partial interface, but that's not allowed by the IDL grammar.

There used to be a [Constructor] there, which was disallowed on partial interfaces in prose, but not by the grammar, and a note about how that was not really OK and needed to be addressed by merging with CSSOM. But the new situation is arguably even worse, because now the IDL can't be parsed at all... And the note about things not being OK was removed, looks like.

@rakina @nordzilla

nordzilla commented 4 years ago

Ah yes. That happened in my PR. Sorry about that.

I had removed the note because it was specific to [Constructor] and no longer seemed relevant.

I'm unclear why it is listed as a partial interface at all if the presence of constructor operations necessitates that the interface is not partial.

I think it would be reasonable to just remove the partial keyword and perhaps leave a new note that explains that the listed functionality only represents what is relevant to Constructable StyleSheets, and not the entirety of the interface.

tabatkins commented 4 years ago

Actually I'm just wondering why constructor() is disallowed on partials. That seems like an arbitrary and unneeded restriction.

domenic commented 4 years ago

What makes it non-arbitrary is that two partials cannot both add constructors. Since only one spec can do so, it really should be the same as the spec that defines the interface.

It would be really ideal if we could upstream this spec into CSSOM itself, as has always been the plan.

tabatkins commented 4 years ago

Two partials can't add the same method with ambiguous signatures, either, but we still let partials define methods. Same with get/set of the same attribute name, etc.

Mentally, partial is just a way to say "mush this body into the existing interface body", and this breaks that model. My question is whether that breakage is doing something useful for spec authors and readers, or is it just an arbitrary legacy restriction? It looks like it's the latter, to me.

bzbarsky commented 4 years ago

but we still let partials define methods.

We do not let partials define methods with the same name as the main interface or as other partials.

That is, partials cannot change the behavior of a method defined in some other partial or the main interface by adding overloads to it.

Mentally, partial is just a way to say "mush this body into the existing interface body"

No, it's actually not, in the spec's model.... In the spec's model, partials are a way to define extensions to interfaces without modifying the main spec defining the interface. They exist to a large extent as a process-workaround feature for standards organizations with processes that do not allow easy modification of "finalized standards" to add functionality. That means partials come with various restrictions on what they can do, compared to the primary interface definition.

Making an object constructible is a pretty fundamental change to the way the object behaves, so it's restricted to non-partials. We could allow constructors in partials, but then we'd have a problem any time a partial wanted to add a constructor while the main interface or some other partial already had one, just like we do for normal operations. Except for normal operations partials tend to just pick non-colliding method names as needed. ;)

tabatkins commented 4 years ago

Ooh, good thing you posted this literally just now, as I was about to leave another comment complaining that constructor() can already have overloads, so what really was the deal.

But if partials aren't allowed to add overloads to an existing method, then this restriction is more reasonable. An interface with no constructor() still has a constructor, it just takes no arguments and throws immediately.