Re: [whatwg/fetch] Add support for compression dictionary transport (PR #1854)

@fred-wang commented on this pull request.



> +
+   <li><p>If <var>dictionaryValue</var>["<code>type</code>"] <a for=map>exists</a> and its
+   <a>bare item</a> is not an <a>implementation-defined</a> supported dictionary type, then return
+   <var>response</var>.
+
+   <li><p>If <var>dictionaryValue</var>["<code>id</code>"] <a for=map>exists</a> and its
+   <a>bare item</a>'s <a for=string>length</a> is greater than 1024, then return
+   <var>response</var>.
+
+   <li>
+    <p>If <var>dictionaryValue</var>["<code>match-dest</code>"] <a for=map>exists</a>:
+
+    <ol>
+     <li><p>Let <var>matchDestList</var> be <var>dictionaryValue</var>["<code>match-dest</code>"][0].
+
+     <li><p>For each <var>dest</var> of <var>matchDestList</var>: if <var>dest</var>'s <a>bare item</a>

So per what you said earlier, condition this and the next item on whether matchDestList is nonempty?

> +   "<code>dictionary</code>", and <var>response</var>'s <a for=response>header list</a>.
+
+   <li><p>If <var>dictionaryValue</var> is null or <var>dictionaryValue</var>["<code>match</code>"]
+   does not <a for=map>exist</a>, then return <var>response</var>.
+
+   <li><p>Let <var>compressionDictionaryCache</var> be the result of
+   <a>determining the compression-dictionary cache partition</a> given <var>request</var>.
+
+   <li><p>If <var>compressionDictionaryCache</var> is null, then return <var>response</var>.
+
+   <li><p>Let <var>pattern</var> be the result of
+   <a for=/>creating a URL pattern</a> given the bare item of <var>dictionaryValue</var>["<code>match</code>"],
+   the <a lt="URL serializer">serialization</a> of <var>request</var>'s <a for=request>current URL</a>,
+   and an empty map.
+
+   <li><p>If <var>pattern</var> is failure or <var>pattern</var> <a for=/>has regexp groups</a>,

> FYI, I updated the PR to fail registration when id is invalid to match the test.
> For unsupported dests, this likely means we don't want to register them at all since there is nothing they could match and we don't want them to be treated like a wildcard.

OK I think these address my remaining concerns regarding mismatch between spec and tests. For match-dest, I believe the match-dest change is still pending, but probably you can just do an early check for the emptiness of match-dest rather than remembering the initial state (see my other comment above).

> +   <li><p>If <var>dictionaryValue</var> is null or <var>dictionaryValue</var>["<code>match</code>"]
+   does not <a for=map>exist</a>, then return <var>response</var>.
+
+   <li><p>If <var>dictionaryValue</var>["<code>type</code>"] <a for=map>exists</a> and its
+   <a>bare item</a> is not an <a>implementation-defined</a> supported dictionary type, then return
+   <var>response</var>.
+
+   <li><p>If <var>dictionaryValue</var>["<code>id</code>"] <a for=map>exists</a> and its
+   <a>bare item</a>'s <a for=string>length</a> is greater than 1024, then <a for=map>remove</a>
+   <var>dictionaryValue</var>["<code>id</code>"].
+
+   <li>
+    <p>If <var>dictionaryValue</var>["<code>match-dest</code>"] <a for=map>exists</a>:
+
+    <ol>
+     <li><p>Let <var>matchDestList</var> be <var>dictionaryValue</var>["<code>match-dest</code>"][0].

OK, I see thanks

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1854#pullrequestreview-4455520636
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1854/review/4455520636@github.com>