feat(parse5): change parser/tokenizer methods to be protected

[43081j]

Some projects in the wild depend on extending Parser and Tokenizer by overriding certain methods and using internals we don't publish on our public API.

This just moves all the private members to be protected so those projects can upgrade to 7.x.

Not everything should be protected but it seems our current class members all fit the criteria.

Fixes #992

i think this should be fine but let me know your thoughts @fb55

[43081j]

@fb55 any chance you can help with getting this over the line and releasing it?

it'll unblock some work in salesforce and google im trying to sort out

[wooorm]

This just moves all the private members to be protected so those projects can upgrade to 7.x.

I’d personally prefer to drop the privates. I already need to include ±100 ts-expect-errors because everything’s private on the classes, while it actually works at runtime.
Would that be acceptable?

[43081j]

@wooorm do you mean you're calling those private (now protected in this branch) methods from outside?

i.e. parser._somePrivateMethod()

as opposed to calling it within a subclass (this._somePrivateMethod())

if thats the case, it suggests these are public methods really, yeah. maybe you could help with an example of why you need to call those methods publicly

[wooorm]

See https://github.com/syntax-tree/hast-util-raw.
It’s a complex project that tightly integrates with how p5 works.
It uses the parser to pass both nodes through and strings of HTML, while keeping positional info and other data on objects around.
This is a particular problem that arises with markdown as you can both have nice nodes, and strings of HTML inside/around them.

[43081j]

interesting!

i wonder if it'd make more sense that you implement that logic as a custom parser? you could inherit Parser to consume your custom data and call the exact same methods but from within your equivalent parse method.

something like that.

but ofc thats a big job so its unlikely even if you agree, it'd get done any time soon i think. so im not sure... we can make them public so you don't need to refactor, or we can keep them protected and aim to one day rework your code

[43081j]

@fb55 can you help me decide what to do here?

i don't want this to go stale as i have third party PRs waiting on it

looks like we need to make them protected or public, i don't mind which. public will help @wooorm out

[fb55]

My only holdback against public is that that would make the docs harder to consume, but we could fix that using @internal annotations. Besides that, feel free to go ahead with this change.

[43081j]

@wooorm are you ok if we go ahead with protected for now and do the larger public change later? since your code isn't blocked by it at least

i feel like there's probably a cleaner solution than just making everything public for you too, e.g. exposing more helpful public APIs you can use

[wooorm]

I think it doesn’t break my code, indeed. So indeed, this does not affect me, and that’s okay!

[43081j]

@fb55 not sure how you've been publishing releases, so would you mind getting one published some day? no rush. or can let me know what process you've been following and i'll do the same

[divmain]

I went to open an issue/PR asking for exactly this and discovered the exact change I was hoping for had just recently been merged. Kudos and thanks!

[43081j]

@divmain thats because i also had a branch lying around for updating LWC's parse5! 😂

i still have some other changes this will unblock in lwc though, so ill open a pr just for those