fix(core/dfn): treat internal slots as IDL attributes #3646

marcoscaceres · 2021-06-21T09:46:45Z

Closes https://github.com/w3c/respec/issues/3644

Require that all internal slots be "for" something.

src/core/dfn.js

saschanaz · 2021-06-21T10:51:49Z

src/core/dfn.js

+ * @param {string} title
+ * @param {HTMLElement} dfn
+ */
+function processInternalSlotsAsIDL(title, dfn) {


Although webref gives IDL attribute and internal slots the same type: "attribute", an internal slot is not an IDL attribute. So I would rename this to something like associateInternalSlotWithIDL.

see also my remark at https://github.com/w3c/respec/issues/3644#issuecomment-864894696

dontcallmedom · 2021-06-21T12:45:55Z

to close #3644 ReSpec would also need to flag as error any definition that doesn't match the established taxonomy

marcoscaceres · 2021-06-22T13:39:44Z

to close #3644 ReSpec would also need to flag as error any definition that doesn't match the established taxonomy

Will add... I'll also do automatic checking for [[\method]](that, people, like), and set the type to "method".

Won't export by default.

src/core/dfn.js

sidvishnoi · 2021-06-22T14:18:06Z

src/core/dfn.js

+      "Use a `data-dfn-for` attribute to associate this dfn with a WebIDL interface.";
+    showError(msg, name, { hint, elements: [dfn] });
+  }
+  dfn.dataset.dfnType = "attribute";


This is something that will be fixed in follow-up, right? i.e., a new internalSlot type and internalSlotMethod maybe?

I don't think so.. See, for example:
https://respec.org/xref/?term=%5B%5BCollectFromCredentialStore%5D%5D%28origin%2C+options%2C+sameOriginWithAncestors%29

Will the existing tools throw if we introduce a new dfnType here?

re new type, discussion started in speced/spec-dfn-contract#1

src/core/dfn.js

saschanaz · 2021-06-23T09:54:41Z

src/core/dfn.js

@@ -60,8 +60,7 @@ function processAsInternalSlot(title, dfn) {
  // Assure that it's data-dfn-for= something.
  if (!dfn.dataset.dfnFor) {
    const msg = `Internal slot "${title}" must be associated with a WebIDL interface.`;
-    const hint =
-      "Use a `data-dfn-for` attribute to associate this dfn with a WebIDL interface.";
+    const hint = docLink`Use a ${"[data-dfn-for]"} attribute to associate this dfn with a WebIDL interface.`;


Wow, interesting!

We will eventually need a way to actually verify this... but pretty cool if we can do that!

src/core/dfn.js

src/core/inline-idl-parser.js

sidvishnoi · 2021-06-23T11:45:06Z

src/core/inline-idl-parser.js

      data-lt="${lt}"
-      ><code>[[${identifier}]]</code></a
+      ><code>[[${identifier}]]${isMethod ? html`(${argsHtml})` : null}</code></a


Can probably merge this into argsHtml above.

sidvishnoi · 2021-06-23T11:46:29Z

tests/spec/core/dfn-spec.js

+      const ops = makeStandardOps(null, body);
+      const doc = await makeRSDoc(ops);


This is repeated (with same args) in each test here. Can probably move it into a beforeAll?

It's specific to this particular describe() group, so beforeAll is probably not ideal... wish they had a "before you run these tests".

I think beforeAll is scoped to desribe() blocks

Ok, will try. The docs said they beforeAll was global.

sidvishnoi · 2021-06-23T11:47:01Z

tests/spec/core/dfn-spec.js

+      const doc = await makeRSDoc(ops);
+      const dfn = doc.getElementById("method");
+      expect(dfn.textContent.trim()).toBe(
+        "[[I am_a method]](I, really, ...am)"


Are spaces acceptable in internal slot names? Any real world examples?

there are no rules, which is another reason why I'm not very comfortable with labeling them as attributes; there are real-world examples of spaces in internal slots, like [[unique id]] (term.startsWith('[[') && term.match(/ .+\]/) lists them all on https://respec.org/xref/filter/)

(I should've checked given I wrote the filter tool 😆)

Though, at least no internalSlot-method with spaces.

Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>

src/core/dfn.js

Specifically, after w3c/respec#3646, we are expected to use {{Interface[[InternalSlot]]}} rather than just {{[[InternalSlot]]}}. The changes in this commit are somewhat larger due to the need to keep tidy happy.

As required by new respec release see also https://github.com/w3c/respec/pull/3646

As now required by ReSpec per https://github.com/w3c/respec/pull/3646

As now required in ReSpec, per https://github.com/w3c/respec/pull/3646

dontcallmedom · 2021-07-06T07:42:57Z

Won't export by default.

it looks like this didn't make it in the pull request - i.e. internal slots seems to be exported by default - @marcoscaceres ?

marcoscaceres · 2021-07-07T03:49:04Z

Weird... checking...

marcoscaceres · 2021-07-07T04:52:57Z

Ah, found it... it's following "the contract" (export everything except "dfn")... I'll fix this as part of https://github.com/w3c/respec/pull/3667/

Cc you there!

fix(core/dfn): treat internal slots as IDL

a836e8a

marcoscaceres commented Jun 21, 2021

View reviewed changes

src/core/dfn.js Outdated Show resolved Hide resolved

Update src/core/dfn.js

700a8e1

marcoscaceres requested review from sidvishnoi and saschanaz June 21, 2021 09:49

marcoscaceres changed the title ~~fix(core/dfn): treat internal slots as IDL~~ fix(core/dfn): treat internal slots as IDL attributes Jun 21, 2021

saschanaz approved these changes Jun 21, 2021

View reviewed changes

Review feedback

0d5c2d8

dontcallmedom mentioned this pull request Jun 21, 2021

Ignore dfns that have an invalid type w3c/reffy#659

Merged

marcoscaceres added 2 commits June 22, 2021 23:40

Merge branch 'develop' into internal_slot

204af22

Merge branch 'develop' into internal_slot

fb81bb0

sidvishnoi approved these changes Jun 22, 2021

View reviewed changes

marcoscaceres added 6 commits June 23, 2021 19:27

v26.9.0

04ee056

Support methods for internal slots too

aeeea2d

stray debugger

500895f

s/firstTitle/linkingText

45cbbaa

Merge branch 'develop' into internal_slot

074d0d9

use doclink

a665899

saschanaz reviewed Jun 23, 2021

View reviewed changes

sidvishnoi reviewed Jun 23, 2021

View reviewed changes

sidvishnoi self-requested a review June 23, 2021 11:49

marcoscaceres and others added 4 commits June 23, 2021 23:24

Update src/core/dfn.js

c5d8632

Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>

Update src/core/dfn.js

a12748f

Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>

Update src/core/inline-idl-parser.js

bf914ed

Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>

Update src/core/inline-idl-parser.js

9e0cf9d

Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>

marcoscaceres mentioned this pull request Jun 24, 2021

Consider adding internal slots to taxonomy speced/spec-dfn-contract#1

Open

marcoscaceres added 2 commits June 24, 2021 13:18

linking tests

6b235d0

add xref tests

cd0468a

marcoscaceres added 2 commits June 24, 2021 17:49

Review comment

061a6df

Add validation test

e57dbeb

sidvishnoi reviewed Jun 24, 2021

View reviewed changes

src/core/dfn.js Show resolved Hide resolved

marcoscaceres added 4 commits June 25, 2021 19:57

vardiadic support

f029503

Fixup tests

567761c

fixup lint

c8c99c4

revert accidental changes to basic.html

2e7e714

marcoscaceres merged commit 06eeced into develop Jun 28, 2021

marcoscaceres deleted the internal_slot branch June 28, 2021 09:21

rakuco mentioned this pull request Jun 29, 2021

Adapt to ReSpec's new syntax for internal slots. w3c/screen-wake-lock#321

Merged

dontcallmedom referenced this pull request in w3c/mediacapture-main Jul 5, 2021

Set scope of internal slots

e4d6ef9

As required by new respec release see also https://github.com/w3c/respec/pull/3646

dontcallmedom referenced this pull request in w3c/webrtc-pc Jul 5, 2021

Associate internal slots with their interfaces

93e908d

As now required by ReSpec per https://github.com/w3c/respec/pull/3646

dontcallmedom referenced this pull request in w3c/webrtc-pc Jul 5, 2021

Associate internal slots with their interfaces

e2f446b

As now required by ReSpec per https://github.com/w3c/respec/pull/3646

dontcallmedom referenced this pull request in w3c/mediacapture-output Jul 5, 2021

Assign scope to internal slots

b4a347a

As now required in ReSpec, per https://github.com/w3c/respec/pull/3646

dontcallmedom referenced this pull request in w3c/mediacapture-fromelement Jul 5, 2021

Assign scope to internal slots

9f0e228

As now required in ReSpec, per https://github.com/w3c/respec/pull/3646

dontcallmedom mentioned this pull request Jul 5, 2021

Assign scope to internal slots w3c/mediacapture-fromelement#90

Merged

marcoscaceres restored the internal_slot branch July 26, 2021 00:35

marcoscaceres deleted the internal_slot branch July 26, 2021 00:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core/dfn): treat internal slots as IDL attributes #3646

fix(core/dfn): treat internal slots as IDL attributes #3646

marcoscaceres commented Jun 21, 2021 •

edited

Loading

saschanaz Jun 21, 2021

dontcallmedom Jun 21, 2021

dontcallmedom commented Jun 21, 2021

marcoscaceres commented Jun 22, 2021

sidvishnoi Jun 22, 2021 •

edited

Loading

marcoscaceres Jun 23, 2021

saschanaz Jun 23, 2021

dontcallmedom Jun 23, 2021

saschanaz Jun 23, 2021

marcoscaceres Jun 23, 2021

sidvishnoi Jun 23, 2021

sidvishnoi Jun 23, 2021

marcoscaceres Jun 24, 2021

sidvishnoi Jun 24, 2021

marcoscaceres Jun 25, 2021

sidvishnoi Jun 23, 2021

dontcallmedom Jun 23, 2021

sidvishnoi Jun 23, 2021

sidvishnoi Jun 23, 2021

dontcallmedom commented Jul 6, 2021

marcoscaceres commented Jul 7, 2021

marcoscaceres commented Jul 7, 2021

		const ops = makeStandardOps(null, body);
		const doc = await makeRSDoc(ops);

fix(core/dfn): treat internal slots as IDL attributes #3646

fix(core/dfn): treat internal slots as IDL attributes #3646

Conversation

marcoscaceres commented Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dontcallmedom commented Jun 21, 2021

marcoscaceres commented Jun 22, 2021

sidvishnoi Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dontcallmedom commented Jul 6, 2021

marcoscaceres commented Jul 7, 2021

marcoscaceres commented Jul 7, 2021

marcoscaceres commented Jun 21, 2021 •

edited

Loading

sidvishnoi Jun 22, 2021 •

edited

Loading