-
Notifications
You must be signed in to change notification settings - Fork 35
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
Remove opal-activesupport dependency? #135
Comments
@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. |
Might be a fair point. How small is small though? And will opal-activesupport stay that way? Maybe as a compromise, getting rid of I know some files already do a granular require but the top level one is there. |
@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. |
again the suggestion of either a basic vs. full version would answer this, or somehow making it configurable. |
For this one I don't think you need a basic or full react.gem. it's just a sent from my mobile device
|
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. |
This issue was moved to ruby-hyperloop/hyper-react#135 |
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.
The text was updated successfully, but these errors were encountered: