Add Imperative Slot API by mfreed7 · Pull Request #6561 · whatwg/html

mfreed7 · 2021-04-06T23:36:16Z

This is a fresh PR, with updates from #5483. I don't have permissions to update that PR.

The explainer for this feature is here: https://github.com/w3c/webcomponents/blob/gh-pages/proposals/Imperative-Shadow-DOM-Distribution-API.md
The issue discussion is here: 3534
There is a corresponding Pull Request for the DOM spec that goes along with this PR.

At least two implementers are interested (and none opposed):
- Chromium tracking bug is here, with the original implementation tracker here.
- The implementation is based on what was agreed in the last F2F Meeting.
Tests are written and can be reviewed and commented upon at:
- In WPT: https://wpt.fyi/results/shadow-dom/slots-imperative-slot-api.tentative.html?label=master&label=experimental&aligned&q=slots-imperative-slot-api.tentative.html
- In WPT: https://wpt.fyi/results/shadow-dom/slots-imperative-api-slotchange.tentative.html?label=master&label=experimental&aligned
- More WPTs to be implemented as part of the Chromium implementation.
Implementation bugs are filed:
- Chrome: https://crbug.com/1196842
- Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1705141
- Safari: https://bugs.webkit.org/show_bug.cgi?id=218692

/acknowledgements.html ( diff )
/infrastructure.html ( diff )
/scripting.html ( diff )

/acknowledgements.html ( diff )
/dom.html ( diff )
/images.html ( diff )
/infrastructure.html ( diff )
/scripting.html ( diff )

domenic

Generally looks good with one small suggestion.

Tests are written and can be reviewed and commented upon at:

Let's be sure to at least have a tracking bug (Chromium is fine, if you intend to do it soon) to test the changes made in the process of getting this merged. So far I note:

Duplicate handling, wherever we land on that
Removal of not-a-child-of-shadow-host check

source

mfreed7 · 2021-04-07T22:12:38Z

Generally looks good with one small suggestion.

Tests are written and can be reviewed and commented upon at:

Let's be sure to at least have a tracking bug (Chromium is fine, if you intend to do it soon) to test the changes made in the process of getting this merged. So far I note:

Duplicate handling, wherever we land on that

Removal of not-a-child-of-shadow-host check

Glad it looks ok! I've filed a Chromium bug, with your suggestions plus a few of my own. Other suggestions for test blind spots to close are appreciated.

source

mfreed7

Ok, edits made. See this comment for details.

source

annevk · 2021-04-13T10:31:42Z

(@rniwa if you find specifications using the older style to reference dictionary members, please file an issue. It should all migrate to the ordered maps syntax.)

mfreed7 · 2021-04-14T23:10:26Z

Thanks everyone! I'm in the process of implementing on Chromium, and I think I found one small thing that needs to be modified. I'll open a fresh PR.

Explainer: https://github.com/WICG/webcomponents/blob/gh-pages/proposals/Imperative-Shadow-DOM-Distribution-API.md. Corresponding HTML PR: whatwg/html#6561. Closes #860 by superseding it. Co-authored-by: Yu Han <yuzhehan@yuzhehan-macbookpro.roam.corp.google.com>

The original implementation of the imperative slot distribution API was done before the final spec PRs landed. In the process of landing those PRs, several changes were made to the way the API works. Primarily, there are two changes: 1. The "auto" slotAssignment mode was renamed to "named". 2. The "linkage" that is created by HTMLSlotElement.assign() was made more permanent. Previously, moving either the <slot> or the assigned node around in the tree (or across documents) would "break" the linkage. Now, the linkage is more permanent, and the only way to break it is through another call to .assign(). See [1] for the chromestatus entry, [2] for the intent to ship, [3], [4], and [5] for the spec PRs, and [6]/[7] for the landed spec. [1] https://chromestatus.com/feature/4979822998585344 [2] https://groups.google.com/a/chromium.org/g/blink-dev/c/6U78F3KWJ78 [3] whatwg/html#6561 [4] whatwg/html#6585 [5] whatwg/dom#966 [6] https://dom.spec.whatwg.org/#find-slotables [7] https://html.spec.whatwg.org/#dom-slot-assign Fixed: 1196842 Fixed: 1067153 Bug: 1067157 Change-Id: I0ee71043c23f3b49a1461296d722045f06eca540

The original implementation of the imperative slot distribution API was done before the final spec PRs landed. In the process of landing those PRs, several changes were made to the way the API works. Primarily, there are two changes: 1. The "auto" slotAssignment mode was renamed to "named". 2. The "linkage" that is created by HTMLSlotElement.assign() was made more permanent. Previously, moving either the <slot> or the assigned node around in the tree (or across documents) would "break" the linkage. Now, the linkage is more permanent, and the only way to break it is through another call to .assign(). See [1] for the chromestatus entry, [2] for the intent to ship, [3], [4], and [5] for the spec PRs, and [6]/[7] for the landed spec. [1] https://chromestatus.com/feature/4979822998585344 [2] https://groups.google.com/a/chromium.org/g/blink-dev/c/6U78F3KWJ78 [3] whatwg/html#6561 [4] whatwg/html#6585 [5] whatwg/dom#966 [6] https://dom.spec.whatwg.org/#find-slotables [7] https://html.spec.whatwg.org/#dom-slot-assign Fixed: 1196842 Fixed: 1067153 Bug: 1067157 Change-Id: I0ee71043c23f3b49a1461296d722045f06eca540 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824763 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/master@{#874413}

The original implementation of the imperative slot distribution API was done before the final spec PRs landed. In the process of landing those PRs, several changes were made to the way the API works. Primarily, there are two changes: 1. The "auto" slotAssignment mode was renamed to "named". 2. The "linkage" that is created by HTMLSlotElement.assign() was made more permanent. Previously, moving either the <slot> or the assigned node around in the tree (or across documents) would "break" the linkage. Now, the linkage is more permanent, and the only way to break it is through another call to .assign(). See [1] for the chromestatus entry, [2] for the intent to ship, [3], [4], and [5] for the spec PRs, and [6]/[7] for the landed spec. [1] https://chromestatus.com/feature/4979822998585344 [2] https://groups.google.com/a/chromium.org/g/blink-dev/c/6U78F3KWJ78 [3] whatwg/html#6561 [4] whatwg/html#6585 [5] whatwg/dom#966 [6] https://dom.spec.whatwg.org/#find-slotables [7] https://html.spec.whatwg.org/#dom-slot-assign Fixed: 1196842 Fixed: 1067153 Bug: 1067157 Change-Id: I0ee71043c23f3b49a1461296d722045f06eca540 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824763 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/master@{#874413}

smaug---- · 2021-04-25T21:18:48Z

So did we get any tests and does any implementation pass them?
It seems like the implementation in Blink
https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/html/html_slot_element.idl;l=35
is quite different to the spec https://html.spec.whatwg.org/#htmlslotelement
(see different params to assign())
The tests in web-platform-tests/wpt#28521 don't look valid to me.

…s, a=testonly Automatic update from web-platform-tests Implement imperative slotting API changes The original implementation of the imperative slot distribution API was done before the final spec PRs landed. In the process of landing those PRs, several changes were made to the way the API works. Primarily, there are two changes: 1. The "auto" slotAssignment mode was renamed to "named". 2. The "linkage" that is created by HTMLSlotElement.assign() was made more permanent. Previously, moving either the <slot> or the assigned node around in the tree (or across documents) would "break" the linkage. Now, the linkage is more permanent, and the only way to break it is through another call to .assign(). See [1] for the chromestatus entry, [2] for the intent to ship, [3], [4], and [5] for the spec PRs, and [6]/[7] for the landed spec. [1] https://chromestatus.com/feature/4979822998585344 [2] https://groups.google.com/a/chromium.org/g/blink-dev/c/6U78F3KWJ78 [3] whatwg/html#6561 [4] whatwg/html#6585 [5] whatwg/dom#966 [6] https://dom.spec.whatwg.org/#find-slotables [7] https://html.spec.whatwg.org/#dom-slot-assign Fixed: 1196842 Fixed: 1067153 Bug: 1067157 Change-Id: I0ee71043c23f3b49a1461296d722045f06eca540 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824763 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/master@{#874413} -- wpt-commits: db1be04691e1b5fe58e186d682d26c69d282cbe0 wpt-pr: 28521

annevk · 2021-04-26T05:40:25Z

The initial proposal did use a sequence but that got changed during review, but I guess the implementation was never aligned?

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1202591.

mfreed7 · 2021-04-26T16:55:51Z

Oops, thanks! I guess the change from sequence to variadic was made before I started editing the spec PR. Thanks for bringing this up and filing that bug. I'll get the Chromium implementation changed, and as part of that, I'll update the WPTs.

…s, a=testonly Automatic update from web-platform-tests Implement imperative slotting API changes The original implementation of the imperative slot distribution API was done before the final spec PRs landed. In the process of landing those PRs, several changes were made to the way the API works. Primarily, there are two changes: 1. The "auto" slotAssignment mode was renamed to "named". 2. The "linkage" that is created by HTMLSlotElement.assign() was made more permanent. Previously, moving either the <slot> or the assigned node around in the tree (or across documents) would "break" the linkage. Now, the linkage is more permanent, and the only way to break it is through another call to .assign(). See [1] for the chromestatus entry, [2] for the intent to ship, [3], [4], and [5] for the spec PRs, and [6]/[7] for the landed spec. [1] https://chromestatus.com/feature/4979822998585344 [2] https://groups.google.com/a/chromium.org/g/blink-dev/c/6U78F3KWJ78 [3] whatwg/html#6561 [4] whatwg/html#6585 [5] whatwg/dom#966 [6] https://dom.spec.whatwg.org/#find-slotables [7] https://html.spec.whatwg.org/#dom-slot-assign Fixed: 1196842 Fixed: 1067153 Bug: 1067157 Change-Id: I0ee71043c23f3b49a1461296d722045f06eca540 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824763 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/master@{#874413} -- wpt-commits: db1be04691e1b5fe58e186d682d26c69d282cbe0 wpt-pr: 28521

mfreed7 added 3 commits April 6, 2021 15:44

Cherry pick whatwg#5483

f6e4f49

Make requested changes

77fbcc9

Fix syntax errors

813369b

mfreed7 mentioned this pull request Apr 6, 2021

Add Imperative Slot API whatwg/dom#966

Merged

3 tasks

domenic reviewed Apr 7, 2021

View reviewed changes

source Outdated Show resolved Hide resolved

rniwa reviewed Apr 8, 2021

View reviewed changes

source Show resolved Hide resolved

rniwa reviewed Apr 8, 2021

View reviewed changes

source Show resolved Hide resolved

Implement weak pointers and remove strict shadow root check

d0561f7

mfreed7 commented Apr 9, 2021

View reviewed changes

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

Moved weak ref stuff to a note

793136e

domenic approved these changes Apr 12, 2021

View reviewed changes

domenic added addition/proposal New features or enhancements topic: shadow Relates to shadow trees (as defined in DOM) labels Apr 12, 2021

Nits and further Bikeshed metadata additions while in the area

cb38d44

domenic merged commit 24a6d00 into whatwg:main Apr 14, 2021

mfreed7 mentioned this pull request Apr 14, 2021

Clear manual slot assignment when re-assigning #6585

Merged

chromium-wpt-export-bot mentioned this pull request Apr 15, 2021

Implement imperative slotting API changes web-platform-tests/wpt#28521

Merged

orstavik mentioned this pull request Nov 9, 2021

Make HTMLSlotElement.assign() accept sequence #7321

Closed

Peeja mentioned this pull request May 25, 2022

Document ShadowRootInit.slotAssignment mdn/content#16666

Closed

4 tasks

josepharhar mentioned this pull request Oct 4, 2022

Web Components web-platform-tests/interop#181

Closed

Westbrook mentioned this pull request Oct 12, 2022

Imperative Slot Assignment web-platform-tests/interop#207

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Imperative Slot API#6561

Add Imperative Slot API#6561
domenic merged 6 commits intowhatwg:mainfrom
mfreed7:imperative_api

mfreed7 commented Apr 6, 2021 •

edited by pr-preview bot

Loading

Uh oh!

domenic left a comment

Uh oh!

Uh oh!

mfreed7 commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

mfreed7 left a comment

Uh oh!

Uh oh!

Uh oh!

annevk commented Apr 13, 2021

Uh oh!

mfreed7 commented Apr 14, 2021

Uh oh!

smaug---- commented Apr 25, 2021 •

edited

Loading

Uh oh!

annevk commented Apr 26, 2021

Uh oh!

mfreed7 commented Apr 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

Conversation

mfreed7 commented Apr 6, 2021 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This is a fresh PR, with updates from #5483. I don't have permissions to update that PR.

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mfreed7 commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

mfreed7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

annevk commented Apr 13, 2021

Uh oh!

mfreed7 commented Apr 14, 2021

Uh oh!

smaug---- commented Apr 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

annevk commented Apr 26, 2021

Uh oh!

mfreed7 commented Apr 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

mfreed7 commented Apr 6, 2021 •

edited by pr-preview bot

Loading

smaug---- commented Apr 25, 2021 •

edited

Loading