Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

Allow better code optimization #210

Closed
piuccio opened this issue Sep 22, 2016 · 8 comments
Closed

Allow better code optimization #210

piuccio opened this issue Sep 22, 2016 · 8 comments

Comments

@piuccio
Copy link

piuccio commented Sep 22, 2016

I finally managed to include preact-compat with modules: true (jsnext: true) and I noticed that there's almost no gain in the gzip bundle (33 bytes). Nothing gets tree-shaken.

I looked into the code and there's a way to fix this. If you move createClass into it's own file, I can use rollup-plugin-alias to replace that file with an empty function (I'm not using createClass in my code).

Kinda like this

import PropTypes from 'proptypes';
import { ... } from 'preact';
import createClass from './createClass';

//...

export default {
    ...,
    createClass,
        ...
};

I tried doing it in a PR but I didn't know exactly how to decouple createClass from currentComponent.

I reckon a good chunk of preact-compat has to do with providing createClass, what do you think?

@developit
Copy link
Member

Hmm - I should think it's at least mildly severable. I wonder - do you think the default export being an Object like that is causing everything to be referenced?

@piuccio
Copy link
Author

piuccio commented Sep 22, 2016

Yeah, I reckon it's the default object, but I haven't tried to get rid of that.

In that case another, simpler, approach is to have one file exporting only named functions and then index.js importing from it and exporting again named and default export. That way I can use rollup-plugin-alias to point to the first file letting rollup tree-shake correctly.

I'll let you know tomorrow

@developit
Copy link
Member

I've been doing that as well, but then people complained that they couldn't do this when using ES Modules via jsnext:main:

import React from 'preact-compat';

Not sure which way to go. Personally I dislike exporting an identical object as default, it seems a bit odd.

@piuccio
Copy link
Author

piuccio commented Sep 23, 2016

@developit #212 suggests a way to keep compatibility and allow tree shaking. Would be great to see if you or anyone else get similar results in terms of bytes saved.

@developit
Copy link
Member

@piuccio Curious if you're still wanting to make changes here. I know you were able to shake quite a bit out of preact and preact-compat - does that mean you were able to work around it?

@piuccio
Copy link
Author

piuccio commented Oct 3, 2016

Hi, organising the code differently would still allow to save extra bytes, but I'm not obsessed by it anymore. After all preact is not very big to start with.

Do what's more comfortable for you in terms of maintenability, code reading and things like that. I wouldn't be upset if this gets closed and the code stays as it is

@developit
Copy link
Member

@piuccio Alrighty. For what it's worth, I'm working on removing as many couplings as possible - previously, Component couldn't be removed because there were type checks that needed a reference to it. Dropping Component means dropping Component.linkState() and a few other things, which might save 500b.

@developit
Copy link
Member

I think we can close this out since we ship an ES bundle now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants