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

Remove Rx from core #465

Closed
jhusain opened this issue Aug 20, 2015 · 14 comments
Closed

Remove Rx from core #465

jhusain opened this issue Aug 20, 2015 · 14 comments

Comments

@jhusain
Copy link
Contributor

jhusain commented Aug 20, 2015

We plan to evaluate removing RX from the core of Falcor. There are several things that motivate this decision:

  1. speed
  2. debuggability
  3. file size

We will be doing some performance analysis over the next few weeks.

@yunong

@yunong
Copy link

yunong commented Aug 20, 2015

Sounds good. I can help with performance analysis as needed.

@trxcllnt
Copy link
Contributor

@jhusain @ktrott how'd we decide on this? should I stop work on integrating RxJS-Next?

@ewinslow
Copy link

+1 on filesize concerns. My eyes went like O_O when I saw how large Falcor is right now.

@ktrott
Copy link
Contributor

ktrott commented Aug 21, 2015

@trxcllnt No decision has been made yet. You should proceed with your work as we're using that to evaluate the performance of the ReactiveX/RxJS Next project so that work is tied to both projects.

@trxcllnt
Copy link
Contributor

The preliminary upgrade to RxJS-Next is done and available in my forks:

https://github.com/trxcllnt/falcor/tree/rx-next
https://github.com/trxcllnt/falcor-router/tree/rx-next

You’ll have to manually build RxJS-Next, since we’re not presently committing the “dist” folder.

For falcor:

~ cd ./node_modules/RxJS-Next
~ npm i

For falcor-router (have to build both falcor and falcor-router’s RxJS-Next packages):

~ cd ./node_modules/RxJS-Next
~ npm i
~ cd ../falcor
~ npm i
~ cd node_modules/RxJS-Next
~ npm i

@ThePrimeagen
Copy link
Contributor

I will be finishing up the get request cycle to have rx completely removed from it. This will be second step towards NoRX in falcor. #506

@tivac
Copy link

tivac commented Oct 9, 2015

I was curious about size savings, so did a quick test using browserify.

-norx files had rx/dist/rx and rx/dist/rx.aggregates excluded from the browserify build.
.min files have been minified with uglify-js
.gz files were gzipped with gzipme.

513,045 falcor.js
278,833 falcor.min.js
276,966 falcor-norx.js
161,130 falcor-norx.min.js
 98,226 falcor.js.gz
 57,624 falcor-norx.js.gz
 54,552 falcor.min.js.gz
 32,932 falcor-norx.min.js.gz

Obviously this doesn't take into account the size of whatever would replace rx.js, but it is interesting IMO.

@ThePrimeagen
Copy link
Contributor

@tivac norx does not remove rx, just rx from the get requesting / batching / across flight deduping. Rx itself is about 100+k when minified. The falcor library should be about 160k minified (i am surprised at the 278k minified, there must be double Rx inclusion).

Our goal would be to remove both path syntax / rx and have our library down to about ~50k, minified. I think this goal is achievable soon.

@tivac
Copy link

tivac commented Oct 9, 2015

@michaelbpaulson That's not from your norx branch. That's me excluding every rx-related module in my browserify config.

The source file is just require("falcor");, then I point browserify at it and see what happens :)

@ThePrimeagen
Copy link
Contributor

The last of the required changes will be in this issue: #604
Once that is complete then we can remove Rx completely.

@ThePrimeagen
Copy link
Contributor

Now Rx itself can be removed from the dependencies. The removal will take a moment due to unit test upgrade.

@tivac
Copy link

tivac commented Nov 6, 2015

Did some new filesize comparisons of falcor@master vs falcor@0.1.14 now that #606 has landed.

  • .min files are compressed with latest Uglify2
  • .jsgz are compressed using gzip-size-cli
  • Sizes calculated by running browserify against a file that simply contains require("falcor");
File Size
falcor-0.1.14.js 499.41 kB
falcor-0.1.14.min.js 272.37 kB
falcor-master.js 250.17 kB
falcor-master.min.js 141.37 kB
falcor-0.1.14.jsgz 93.91 kB
falcor-0.1.14.min.jsgz 52.75 kB
falcor-master.jsgz 51.51 kB
falcor-master.min.jsgz 28.09 kB

About half the size, kudos @michaelbpaulson! 👍

@ThePrimeagen
Copy link
Contributor

@tivac I don't think you are testing the correct file. A simple gulp dist shows falcor.browser.min.js being 68k on master right now. falcor.all.min.js is not the one to test as it includes falcor-router.

@tivac
Copy link

tivac commented Nov 6, 2015

I mentioned my method in the post:

Sizes calculated by running browserify against a file that simply contains require("falcor");

Since we use falcor via browserify that test is at least accurate for our needs!

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

7 participants