Skip to content

Script Modules: Allow scripts to depend on modules - #10

Closed
sirreal wants to merge 2 commits into
trunkfrom
script-modules/allow-as-script-dependency
Closed

Script Modules: Allow scripts to depend on modules#10
sirreal wants to merge 2 commits into
trunkfrom
script-modules/allow-as-script-dependency

Conversation

@sirreal

@sirrealsirreal commented Oct 10, 2024

Copy link
Copy Markdown
Owner

Trac ticket:


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@sirreal
sirrealforce-pushed the script-modules/allow-as-script-dependency branch from 13417a6 to f9c245cCompareOctober 10, 2024 10:24
This allows for script modules to be exposed in the import map
regardless of being a dependency or enqueued.
This will enable scripts to depend on script modules by requesting they
be exposed.
@sirreal
sirrealforce-pushed the script-modules/allow-as-script-dependency branch from f9c245c to 117fe60CompareOctober 10, 2024 10:26
*
* @param string $id The identifier of the script module.
*/
public function expose( string $id ) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new concept in WordPress, so I wanted to share some alternatives to help find the final name:

  • add_to_import_map
  • include_in_import_map

Conceptually, the idea makes perfect sense to me. This method might also be useful when third-party code wants to ensure its dependency is listed in the import map, and they don't want to use all the tooling to generate dependencies for their script modules.

@sirreal

Copy link
Copy Markdown
OwnerAuthor

Superseded by WordPress#8009.

@sirrealsirreal closed this Dec 16, 2024
@sirreal
sirreal deleted the script-modules/allow-as-script-dependency branch December 16, 2024 15:36
sirreal added a commit that referenced this pull request Jun 10, 2026
Reviewer-1 #10 (class-value decode boundary): ReferenceMatcher::class_matches
now folds NUL -> U+FFFD per class token, matching WP_HTML_Tag_Processor::
class_list(). Previously the reference left raw NUL, so it would have
diverged from select() on a class value containing NUL (a reference gap,
not a WP bug). Pinned by five deterministic decode-boundary cases in
self-check.php (NUL->FFFD, trailing NUL, FF separator) that put the two
independent class tokenizers against each other and against select().
Randomized document-side NUL injection is deliberately NOT added: a
hot-path PRNG draw perturbs self-check.php's fixed seed space enough to
surface the known Bug 3, which would first require decoupling self-check
from the unfixed core bugs. Documented as a scoped future improvement in
README 'Known oracle limitations', which also distinguishes the
independent class-value path from the shared get_attribute() attribute
path per the reviewer's sharpening.
Reviewer-2 nitpick (fid sanitization asymmetric on match path):
collect_matches now routes select() fids through TreeCapture::sanitize_fid,
identical to the tree-row and lexbor sides, so a control-char fid can
never produce a false lexbor-divergence (unreachable today; fids are
integers).
Reviewer-3 nitpick: minimize.php header docstring no longer implies
--seed works universally.
Validated: self-check OK (incl. new cases), unpatched detects the 3 bugs,
patched 2000-seed run clean (927 lexbor comparisons), all 3 bugs still
reproduce.
sirreal added a commit that referenced this pull request Jun 10, 2026
A raw CR followed by a decoded 
 renders as CR+LF in the WordPress
tree while the DOM holds LF+LF: preprocessing maps the lone CR to LF
before the character reference decodes to a second LF. The lockstep
matcher consumed that pair through its CRLF-collapse rule, stranded the
second DOM LF, and reported tree-mismatch for a spec-explainable
difference — the same false-positive class the matcher exists to
prevent. Found by adversarial review of the previous commit.
WordPress CR+LF opposite DOM LF is genuinely ambiguous between a
collapsed raw CRLF and a CR-to-LF substitution followed by an agreed
LF, so that one site now branches: lockstep first, since it resolves
the common case without backtracking, with dead ends memoized and a
step budget that fails closed on adversarial inputs.
sirreal added a commit that referenced this pull request Jun 12, 2026
Reviewer-1 #10 (class-value decode boundary): ReferenceMatcher::class_matches
now folds NUL -> U+FFFD per class token, matching WP_HTML_Tag_Processor::
class_list(). Previously the reference left raw NUL, so it would have
diverged from select() on a class value containing NUL (a reference gap,
not a WP bug). Pinned by five deterministic decode-boundary cases in
self-check.php (NUL->FFFD, trailing NUL, FF separator) that put the two
independent class tokenizers against each other and against select().
Randomized document-side NUL injection is deliberately NOT added: a
hot-path PRNG draw perturbs self-check.php's fixed seed space enough to
surface the known Bug 3, which would first require decoupling self-check
from the unfixed core bugs. Documented as a scoped future improvement in
README 'Known oracle limitations', which also distinguishes the
independent class-value path from the shared get_attribute() attribute
path per the reviewer's sharpening.
Reviewer-2 nitpick (fid sanitization asymmetric on match path):
collect_matches now routes select() fids through TreeCapture::sanitize_fid,
identical to the tree-row and lexbor sides, so a control-char fid can
never produce a false lexbor-divergence (unreachable today; fids are
integers).
Reviewer-3 nitpick: minimize.php header docstring no longer implies
--seed works universally.
Validated: self-check OK (incl. new cases), unpatched detects the 3 bugs,
patched 2000-seed run clean (927 lexbor comparisons), all 3 bugs still
reproduce.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sirreal@gziolo