Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "visitor" plugins support #1454

Merged
merged 2 commits into from
Mar 28, 2021
Merged

Add "visitor" plugins support #1454

merged 2 commits into from
Mar 28, 2021

Conversation

TrySound
Copy link
Member

Visitor is a simple pattern which helps to avoid many type checks
and provide both "perItem" and "perItemReverse" functionality without
fragmentation.

The most important case is an ability to define state which in many
plugins specified either on module level or by polluting params.

In this diff I added visit and detachFromParent utilities and refactored
new mergeStyles plugin with it.

Also fixed bug when cdata content is merged into "text" node which is
not always valid.

cc @strarsis @XhmikosR @sk-

Visitor is a simple pattern which helps to avoid many type checks
and provide both "perItem" and "perItemReverse" functionality without
fragmentation.

The most important case is an ability to define state which in many
plugins specified either on module level or by polluting `params`.

In this diff I added visit and detachFromParent utilities and refactored
new mergeStyles plugin with it.

Also fixed bug when cdata content is merged into "text" node which is
not always valid.
@TrySound TrySound requested a review from deepsweet March 27, 2021 15:40
@strarsis
Copy link
Contributor

+1, this is a great addition.

@XhmikosR
Copy link
Contributor

LGTM from a quick look 👍

@TrySound TrySound merged commit 27bef1a into master Mar 28, 2021
@TrySound TrySound deleted the visitor branch March 28, 2021 08:20
@strarsis
Copy link
Contributor

@TrySound: So after using this plugin type I found an improvement:
After the visitor visited all nodes it should call the plugins that use that kind of pattern a last time so the plugin can do manipulation after having visited all DOM nodes. This is necessary sometimes, e.g. when the plugin first has to collect all <use> and then, afterwards, query all elements that are referenced.

@TrySound
Copy link
Member Author

TrySound commented May 19, 2021

I don't get it tbh. Visitors are not combined. Each plugin is run with own visitor. This is important for correctness.

Sign up for free to 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.

4 participants