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

Further improve bundle size #50

Closed
trueadm opened this issue Nov 30, 2016 · 9 comments
Closed

Further improve bundle size #50

trueadm opened this issue Nov 30, 2016 · 9 comments

Comments

@trueadm
Copy link
Contributor

trueadm commented Nov 30, 2016

I was looking at the js-framework-benchmark output for Svelte and it got me thinking, take this:

function renderMainFragment ( root, component, target ) {
	var div = document.createElement( 'div' );
	div.className = "jumbotron";
	
	var div1 = document.createElement( 'div' );
	div1.className = "row";
	
	var div2 = document.createElement( 'div' );
	div2.className = "col-md-6";
...
}

Could you not simplify this somewhat, to be:

function elementWithClass ( tag, className ) {
 var elem = document.createElement( tag );
 elem.className = className;

 return elem;
}
function renderMainFragment ( root, component, target ) {
  var div = elementWithClass('div', 'jumbotron');
  var div1 = elementWithClass('div', 'row');
  var div2 = elementWithClass('div', 'col-md-6');
...
}

This is actually something the old Inferno's used to do (would compile vdom to client-side code) with t7. It would look ahead to see if there are many common patterns (like an element with a className) and build dedicated factories for them. Performance actually improved in many cases, as the function for the component was small enough when minified to be inlined by most JavaScript engines.

@Rich-Harris
Copy link
Member

We should definitely do this. As a starting point, helpers like createElement and detachElement would reduce the size a bit (gzip notwithstanding) and could be externalised if necessary. I hadn't thought of custom factories though, that's an interesting idea.

@Ryuno-Ki
Copy link

cloneNode could be helpful here, too.

@emirotin
Copy link

emirotin commented Dec 2, 2016

Was about to suggest the similar optimization. Plucking methods like document.createElement into the variables at the top of the module (like const __createElement = document.createElement.bind(document)) will help the minifier when the components have many nodes.

@chrisdavies
Copy link

Yup. I did a little test locally yesterday, and found about a 20% savings by doing this on a relatively small project.

@emirotin
Copy link

emirotin commented Dec 2, 2016

BTW does it make sense to have elementWithClass actually compatible with React.createElement which is (name, attributes, children)?
Who knows if it will provide some positive side-effect like JSX compatibility?

@trueadm
Copy link
Contributor Author

trueadm commented Dec 2, 2016

@emirotin you want to avoid polymorphic functions like that, that's why Inferno avoids them in core – they result in functions that can't be properly optimised.

@emirotin
Copy link

emirotin commented Dec 2, 2016

Interesting @trueadm thanks for the insight.

@PaulBGD
Copy link
Member

PaulBGD commented Jul 11, 2017

Helpers have been in Svelte for a while, is this issue open for other helpers? Like the suggested elementWithClass and potentially similar ones like elementWithId?

@Rich-Harris
Copy link
Member

Probably slightly trickier to do elementWithClass in Svelte's current form, because creating nodes and adding attributes happens in two separate places (so that hydration can work). So maybe we should close this and revisit it if we ever need to eke out a few extra microseconds 😀

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

No branches or pull requests

6 participants