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

AcornJSX Refactor #19

Merged
merged 5 commits into from
Jul 2, 2017
Merged

AcornJSX Refactor #19

merged 5 commits into from
Jul 2, 2017

Conversation

TroyAlford
Copy link
Owner

@TroyAlford TroyAlford commented Jun 27, 2017

Closes #17, #18, and #20

This is a complete rewrite based on acorn and acorn-jsx - which is the same underlying JSX parser which is used by Babel. As a result, it handles a lot more edge cases. Unfortunately, it also adds something like ~70kb to the size of the minified output. :(

That said - I think it's worth discussion. It already passes all existing unit tests at this point, and adds the additional features of being able to parse JSX with non-string props embedded, such as:

<h1>Title</h1>
<MyCustomElement myArray={[1,2,3]} myObj={{ foo: 'bar' }} /> 

It still does not handle parsing function props (myFn={() => { /* do stuff */ }}), as doing so will require some further thought. It should also throw away props which include symbols (myProp={foo}), as the parser won't recognize the value of those symbols. (Though it may be reasonable to either look for a binding key which matches the symbol name, or something similar?)

I'm opening this PR for review and consideration - but not yet sure that it should be merged, due to the significant increase in package size.

@timsuchanek
Copy link
Contributor

Wow, this is incredible! From the power of the parser this is definitely a nice solution.
I may outline my use case so we can think about how this might be used differently than just directly in the browser.

The use case is, that I'm using gatsby. The latest version, gives you the raw html that is parsed from markdown. We have the requirement however, that we need to insert custom React components into the markdown. To inject components into that html I see this approach the only way.

So the current solution is to put the raw html string into react-jsx-parser and inject the components.
As I said, now I'm using it again with the text/html version of the DOMParser because in our case https://github.com/wooorm/remark creates not closed <img> tags.

However, as Gatsby is already executing the markdown parser at webpack compile time, at that step, a json representation of the virtual dom could be generated - something like the inferno docs are using:
If you for example open https://infernojs.org/docs/guides/what-is-jsx, you will see in the network tab a request to https://infernojs.org/api/markdown?file=/guides/what-is-jsx

I think for our use case generating something like this would be perfect
It would for sure be faster (I did some benchmarking with react-jsx-parser and a 250kb html file - this is a rough breakdown where time was spend:

  1. running DOMParser.parseFromString() ~ 10ms
  2. traversing the generated dom ~ 350ms
  3. time from finished dom traversal to componentDidMount of parent component ~ another 350ms.

So probably the first 350ms could be much faster.

Actually, just using dangerouslySetInnerHTML is the fastest variant - it's all rendered in about 20ms.

So probably for us the most optimal solution would be:

  • Use the powerful acorn compiler for the best compatibility with the html/jsx, but run it in a webpack compile step, don't ship it to the browser
  • Just use a function like this to generate the json representation of the vdom:
function h(type, props, children) {
  return {type, props, children}
}
  • Instead of sending the html string, now send this JSON representation to the client, which then deserializes it, with the usual components object to inject custom components
  • My hope is, that this deserialization should be faster than traversing the dom generated from DOMParser.

To sum it up - we're just going through all this trouble to inject React Components into Markdown.

It would be really interesting to know your use case @TroyAlford , there must be a good reason you're going through all this trouble.

@TroyAlford
Copy link
Owner Author

My use-case is my wiki project, which is all React.

When installed, I want the option for the wiki itself to have a series of custom components specified in a plugin model. Because the wiki already associates metadata with each individual page, and allows HTML-editing as well as WYSIWYG, the benefit of this is that someone who wants to customize their own install of the wiki can now supply custom tags, which automatically get the page metadata bound to them as props.

From this point, the wiki maintainer can do things like create a pre-formatted display, with full React functionality, as an available tag within the markup of the wiki - and whoever is writing the article can simply say <Navigation /> or <Navigation title="Customized Title" /> - and then allow the metadata of the page to inject itself, supplying other props.

From a software perspective, this allows the server-side to ONLY store HTML JSX and JSON - and not worry about having to do any custom parsing or anything, because it simply hands off the set of plugin-injected elements as components and the metadata as bindings - react-jsx-parser does the rest.

@TroyAlford
Copy link
Owner Author

Since you posed that perhaps we should think about multiple use-cases... I wonder if we should consider outputting 2 separate components. One intended to be used browser-side, and only 9kb - but more simplistic functionality... and one intended to be used server-side (though someone is certainly welcome to pay the 80kb size increase to have the power client-side) - which uses Acorn.

This gives us both sets of flexibility - and allows us to handle both complex cases and simple ones, at least until a better solution comes along (such as finding a way to tree-shake a big portion of Acorn).

What do you think of that idea?

@TroyAlford TroyAlford merged commit f83f44d into develop Jul 2, 2017
@TroyAlford TroyAlford deleted the feature/acorn-jsx branch July 2, 2017 02:06
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.

2 participants