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.

Sign up for GitHub

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

Jump to bottom

Create element instances, then resolve to strings #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Create element instances, then resolve to strings #6

wants to merge 3 commits into from

Conversation

nathancahill
Copy link
Collaborator

@nathancahill nathancahill commented Apr 30, 2017 •
edited
Loading

Updated with new approach

This PR changes the behavior of h() from immediately executed functions, to a function that first creates element instances, and then later resolves them to a string. This means that JSX is now executed in a top-down order, instead of executing the most deeply nested function first. This also makes props.children an array of elements (instead of resolved strings), that can be manipulated similar to React.Children.

Consider this JSX:

<Article>
    <Heading>
    <Body>
        <Image />
    </Body>
</Article>

The current execution order is: Image(), Heading(), Body(), Article(). This PR switches that to the "expected" Article(), Heading(), Body(), Image().

An example using new props.children:

const Item = props => (
  <li>
    <h4>{props.title}</h4>
  </li>
);

const List = props => (
  <ul>{props.children.map(child => { child.props.title = 'Overridden'; return child; }  }</ul>
);

console.log(
  <List>
    <Item title="First" />
    <Item title="Second" />
  </List>
);

Output:

<ul>
  <li>Overridden</li>
  <li>Overridden</li>
</ul>

Semi-breaking Changes

  • The public JSX/vhtml API must be explicitly cast to a string. The element object uses toString to output the HTML. innerHTML = <Article> still works because innerHTML casts to a string.

  • Size: + 56 bytes, from 580 bytes to 636.

I believe this makes vhtml more consistent with other JSX implementations like React/Preact. It's certainly much more powerful, at the cost of 56 bytes.

@nathancahill nathancahill force-pushed the features/execution-order branch from 8b46ec0 to 11451ed Compare April 30, 2017 16:02
@nathancahill nathancahill changed the title Switch execution order to follow nesting Create element instances, then resolve to strings Apr 30, 2017
@developit
Copy link
Owner

This is a really slick idea! I'm wondering about performance and compatibility though - thinking this now requires any consumer to cast to a String (at least implicitly) whereas before that was unnecessary. Also for perf there are now intermediary objects being created that could cost (though I've seen this can sometimes have the opposite effect too).

I'm curious - do you think this is a large enough change that it would warrant a second project? I'd love to have a direct jsx-to-string renderer and a more react/preact-compatible stateless string renderer. The latter is interesting since it's really an optimization of preact-render-to-string, which takes in Preact VNode objects and builds up an HTML/XML string.

Thoughts on the two project idea?

@nathancahill nathancahill force-pushed the features/execution-order branch from 11451ed to 26c4842 Compare April 30, 2017 22:26
@nathancahill
Copy link
Collaborator Author

Yeah, I agree that keeping vhtml as a direct-to-string renderer is a good thing. I was already thinking about a 3 project approach, where there's room for a slightly more enhanced build of vhtml on the spectrum from vhtml < new project < Preact.

New project could include the object elements in this PR, and the style={object} logic in Preact, maybe a handful of other things which would still come in around 800 bytes or so.

In theory, a lot of the codebase between vhtml and new project could be shared, it could be structured as an alternate Rollup build.

@developit
Copy link
Owner

Agreed, I had wondered if it would be worth doing second entrypoint in vhtml for that. I'm fine with either approach really. If we split some of the functions up into files, the two could share quite a bit of them.

@nathancahill
Copy link
Collaborator Author

Sounds good to me. Let me take a swing at re-working this PR as a second entrypoint and see how it looks. If it feels too contrived it'll be easy to split apart completely.

@nathancahill
Copy link
Collaborator Author

@developit Do you have any feelings about the other PRs to work towards this goal? Deciding whether to focus my efforts on your project or a new one of my own.

@mikestopcontinues
Copy link

I'm not sure of others, but my use case for vhtml is to be able to have all of React's ease of development without any state whatsoever. (And also an easy upgrade path should the project warrant React in the future.)

So whatever project has this feature is the one I want to use. The same goes for the aforementioned style object helper. If y'all think that's a level-up from vhtml, so be it... But is there anyone using vhtml that wouldn't rather have these features? I can't imagine anyone willing to forgo them.

@AndrewLeedham
Copy link

@nathancahill did you get anywhere with this? Perhaps publishing the 2 projects from the same repo with a shared codebase would be a nice middle-ground?

@AndrewLeedham AndrewLeedham mentioned this pull request Jun 16, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nathancahill @developit @mikestopcontinues @AndrewLeedham