Skip to content
This repository has been archived by the owner on Oct 19, 2018. It is now read-only.

Remove opal-activesupport dependency? #135

Closed
sollycatprint opened this issue Jun 13, 2016 · 9 comments
Closed

Remove opal-activesupport dependency? #135

sollycatprint opened this issue Jun 13, 2016 · 9 comments

Comments

@sollycatprint
Copy link

From @wied03 on March 25, 2016 21:15

React-opal has this as well. If it's possible to remove it (like opal-browser) such that it's only included if higher level code needs it, that might help keep the core GEM lean.

Copied from original issue: zetachang/react.rb#135

@sollycatprint
Copy link
Author

From @ajjahn on March 28, 2016 11:35

@wied03 If opal-activesupport were any where near the size of ruby active support, I'd definitely agree with you. But opal-activesupport is still quite lean. In fact older versions of react.rb implemented a handful of core extensions that I moved into the opal-activesupport gem to avoid multiple implementations of the same core ext.

@sollycatprint
Copy link
Author

From @wied03 on March 28, 2016 15:36

Might be a fair point. How small is small though? And will opal-activesupport stay that way?

Maybe as a compromise, getting rid of require 'active_support' from top_level.rb so that the active support requires are more granular might be a good goal. That way active support can grow but the served script to the browser with React.rb doesn't necessarily grow.

I know some files already do a granular require but the top level one is there.

@sollycatprint
Copy link
Author

From @ajjahn on March 29, 2016 2:31

@wied03 I managed to compile all of active support into ~13k. It is quite possible never versions of active support would implement more or it's ruby counter part. I think you are correct to suggest simply being deliberate in only requiring extensions that are needed. That should protect against a growing size as AS matures.

@sollycatprint
Copy link
Author

From @catmando on March 29, 2016 13:52

again the suggestion of either a basic vs. full version would answer this, or somehow making it configurable.

@sollycatprint
Copy link
Author

From @wied03 on March 29, 2016 13:54

For this one I don't think you need a basic or full react.gem. it's just a
matter of a couple more granular requires.

sent from my mobile device
On Mar 29, 2016 07:52, "Mitch VanDuyn" [email protected] wrote:

again the suggestion of either a basic vs. full version would answer this,
or somehow making it configurable.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
zetachang/react.rb#135 (comment)

@sollycatprint
Copy link
Author

From @ajjahn on March 29, 2016 15:31

For this one I don't think you need a basic or full react.gem. it's just a
matter of a couple more granular requires.

Exactly. AS isn't providing features to the user here. It's just allowing us to avoid recoding the wheel. Really, the more we lean on AS (and only require what which we need), the better. Users who include AS in their projects will benefit from the overlap.

@zetachang
Copy link
Member

zetachang commented Oct 29, 2016

Seems like we currently only use the following modules from AS (all tests are passed too)

require 'active_support/core_ext/object/try'
require 'active_support/core_ext/class/attribute'

So I think it do provide a significant reduction if we start to adopt the granular require approach.

Only drawback is that this will be a breaking change to user who relied on the automatically required AS modules.

@catmando
Copy link
Contributor

yeah so we lets do it at the next semantic release...

@zetachang
Copy link
Member

Tested the master against latest release (0.10.0), the reduction of the built file size is 18% (from 327k to 268k) using script. opal-build -r hyper-react -r opal-activesupport -- hyper-react

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

No branches or pull requests

4 participants