refactor(tokenizer): Introduce events #404

fb55 · 2022-02-11T14:04:29Z

Update the tokenizer to produce events. See #403 for details.

No need to construct objects, when flat parameters suffice

This allows us to start asserting expected values as we walk through the document. This will give us stack traces for all future errors. Improve generate tokenization tests imports

Once again, stack traces will make errors easier to identify

wooorm

This is a lot of changes. Including what seems like refactoring. That make it hard to review.
If possible, can you move part of this work to separate PRs?
Can you point me towards subsets that you want me to review?

fb55 · 2022-02-11T14:55:38Z

packages/parse5-html-rewriting-stream/lib/index.ts

-    protected override _handleToken(token: Token): boolean {
-        if (!super._handleToken(token)) {
-            this.emitRaw(this._getRawHtml(token.location!));
+    protected override emitIfListenerExists(eventName: string, token: SaxToken): boolean {


The SAX parser now handles events, so _handleToken is gone. We can override emitIfListenerExists instead for the same effect.

fb55 · 2022-02-11T14:57:02Z

packages/parse5-sax-parser/lib/index.ts

    protected parserFeedbackSimulator: ParserFeedbackSimulator;
-    private pendingText: CharacterToken | null = null;
+    private pendingText: Text | null = null;


With _handleToken gone, we keep a Text object here for simplicity.

fb55 · 2022-02-11T14:58:36Z

packages/parse5-sax-parser/lib/parser-feedback-simulator.ts


-    constructor(private tokenizer: Tokenizer) {
+    constructor(options: TokenizerOptions, private handler: TokenHandler) {


Instead of passing tokens on after a getNextToken call, the ParserFeedbackSimulator now mirrors tokens received from the tokenizer to a passed handler.

fb55 · 2022-02-11T14:59:41Z

packages/parse5-sax-parser/test/parser-feedback-simulator.test.ts

+    'ParserFeedbackSimulator',
+    'ParserFeedbackSimulator',
+    feedbackPath.pathname,
+    (handler) => new ParserFeedbackSimulator({}, handler).tokenizer


generateTokenizationTests now only needs a tokenizer to be returned.

fb55 · 2022-02-11T15:00:17Z

packages/parse5/lib/parser/index.ts

@@ -1,4 +1,4 @@
-import { Tokenizer, TokenizerMode } from '../tokenizer/index.js';
+import { QueuedTokenizer, TokenizerMode } from '../tokenizer/index.js';


The parser is mostly unchanged — only the wrapped tokenizer is imported.

fb55 · 2022-02-11T15:01:25Z

packages/parse5/lib/tokenizer/index.test.ts

-generateTokenizationTests('tokenizer', 'Tokenizer', dataPath.pathname, ({ errors }) => {
-    const tokenizer = new Tokenizer({
-        sourceCodeLocationInfo: true,
-        onParseError(err): void {


The onParseError method for the tokenizer moved to the handler. This is not a breaking change for anyone not explicitly using the tokenizer (and there are a lot of breaking changes here for those that are).

fb55 · 2022-02-11T15:17:43Z

scripts/generate-parser-feedback-test/index.ts

@@ -40,7 +40,41 @@ function appendToken(dest: Token[], token: Token): void {
    dest.push(token);
 }

-function collectParserTokens(html: string): ReturnType<typeof convertTokenToHtml5Lib>[] {
+function convertTokenToHtml5Lib(token: Token): HtmlLibToken {


convertTokenToHtml5Lib was no longer needed in test/utils/generate-tokenization-tests.ts, and therefore moved here.

fb55 · 2022-02-11T15:18:37Z

test/utils/generate-tokenization-tests.ts

                    initialState: getTokenizerSuitableStateName(initialStateName),
                    initialStateName,
                    lastStartTag: descr.lastStartTag,
-                    expectedErrors: descr.errors || [],
+                    expectedErrors: TestsWithBrokenErrors[descr.description]


Broken tests are now referenced here, instead of when comparing once all errors are gathered.

fb55 · 2022-02-11T15:19:31Z

test/utils/generate-tokenization-tests.ts

@@ -141,7 +164,7 @@ interface TestDescription {
    description: string;
    input: string;
    lastStartTag: string;
-    errors?: string[];


errors was wrongly typed; it should have been a TokenError to begin with.

fb55 · 2022-02-11T15:26:16Z

This is a lot of changes. Including what seems like refactoring. That make it hard to review.
If possible, can you move part of this work to separate PRs?

I have pushed some commits that might help make this more digestible.

I was debating whether or not to include updates to other modules & tests. I felt like they help to illustrate how code looks like after adopting this change, so I kept them for now. But happy to move them to a separate PR if needed.

Can you point me towards subsets that you want me to review?

packages/parse5/lib/tokenizer/index.ts and packages/parse5/lib/tokenizer/queued.ts are the two files that are the most crucial.

The tokenizer unfortunately has a lot of changes due to all emitToken calls being replaced with the one for the specific token type. These changes follow a very consistent pattern and are hopefully easy to follow.

…r-events

wooorm · 2022-02-27T15:06:11Z

sorry, thanks for putting in that work, sorry that I didn’t review this! :(

fb55 · 2022-02-27T20:20:27Z

sorry, thanks for putting in that work, sorry that I didn’t review this! :(

No worries — hope it's okay that I moved forward with this after two weeks.

I have opened #419 to wrap up this refactoring and would appreciate a review there (if you have the time).

fb55 added 14 commits February 7, 2022 08:42

refactor(tokenizer): Introduce events

aa8bf73

Move EOF creation & emission to _emitEOFToken

6efaa6d

Port SAX parser to use tokenizer events

9249638

Rename tokenizer classes

9b59041

Flatten text & eof events

764498e

No need to construct objects, when flat parameters suffice

Shorten callback names

6c9e816

Simplify _emitCurrentCharacterToken

5b7b487

refactor(tokenizer): Move onParseError to handler

4a16907

Use tokenizer events for generateTokenizationTests

a85f632

This allows us to start asserting expected values as we walk through the document. This will give us stack traces for all future errors. Improve generate tokenization tests imports

Use tokenizer events for location-info tests

f212403

Once again, stack traces will make errors easier to identify

Directly emit by token type

3f36ff8

Reintroduce character & eof tokens

a82d4c4

Add abstract class SinglePathHandler

ff0efa0

Remove unused queued tokenizer properties

a28d45d

This was referenced Feb 11, 2022

refactor(parser): Invert event processing #405

Merged

Proposal: Replace the token queue with an event-handler system #403

Closed

wooorm reviewed Feb 11, 2022

View reviewed changes

fb55 added 3 commits February 11, 2022 15:12

Move queued handler to its own file

936a523

Revert unwrapping of currentCharacterToken

be2ab48

Add some comments

8631aa5

fb55 commented Feb 11, 2022

View reviewed changes

fb55 added 2 commits February 26, 2022 11:35

Merge remote-tracking branch 'upstream/master' into refactor/tokenize…

2c411d3

…r-events

Fix comments

d65af65

fb55 merged commit 722f429 into inikulin:master Feb 27, 2022

fb55 deleted the refactor/tokenizer-events branch February 27, 2022 07:35

jmbpwtw pushed a commit to jmbpwtw/parse5 that referenced this pull request Feb 16, 2023

refactor(tokenizer): Introduce events (inikulin#404)

7505d87

Nahom1927 mentioned this pull request Aug 27, 2024

[Snyk] Upgrade parse5 from 7.0.0 to 7.1.2 Nahom1927/docs#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(tokenizer): Introduce events #404

refactor(tokenizer): Introduce events #404

fb55 commented Feb 11, 2022

wooorm left a comment

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022

fb55 Feb 11, 2022 •

edited

Loading

fb55 commented Feb 11, 2022 •

edited

Loading

wooorm commented Feb 27, 2022

fb55 commented Feb 27, 2022


		constructor(private tokenizer: Tokenizer) {
		constructor(options: TokenizerOptions, private handler: TokenHandler) {

		@@ -1,4 +1,4 @@
		import { Tokenizer, TokenizerMode } from '../tokenizer/index.js';
		import { QueuedTokenizer, TokenizerMode } from '../tokenizer/index.js';

refactor(tokenizer): Introduce events #404

refactor(tokenizer): Introduce events #404

Conversation

fb55 commented Feb 11, 2022

wooorm left a 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

fb55 Feb 11, 2022 • edited Loading

Choose a reason for hiding this comment

fb55 commented Feb 11, 2022 • edited Loading

wooorm commented Feb 27, 2022

fb55 commented Feb 27, 2022

fb55 Feb 11, 2022 •

edited

Loading

fb55 commented Feb 11, 2022 •

edited

Loading