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

Node Require() Support #3607

Closed
Brian-McBride opened this issue Apr 26, 2018 · 7 comments
Closed

Node Require() Support #3607

Brian-McBride opened this issue Apr 26, 2018 · 7 comments
Assignees

Comments

@Brian-McBride
Copy link

There is a lot of amazing work done around pipeable operators for tree shaking and reduction of bundled filesize.

I feel it is important to keep support going for importing via require() in NodeJS. Currently Node does not support ES6 imports.

In addition, when working with server-side code, there are different opportunities for speed and performance - especially when using Lamdas, Google Cloud Functions, etc...

module.exports = () => {
  const Observable = require('rxjs/Observable').Observable;
  require('rxjs/add/observable/from');
  require('rxjs/add/operator/concatMap');
  require('rxjs/add/operator/reduce');
  // Do something with your observable...
}

The pattern above can be used to improve serverless cold boot starts. In the case of a Lambda, the less code loaded and executed the faster the function. By placing require statements within inner functions, if your lambda supports X functionality and you only use 30% of that functionality on a particular Lambda run cold boot times can be significantly reduced.

NodeJS will not have proper ES6 import support in any immediate timeframe. Thus supporting the require methods of importing RXJS as well as continuing to provide dynamic additions of operators as needed will continue to enhance the use of RXJS in the serverless environment.

@kwonoj
Copy link
Member

kwonoj commented Apr 26, 2018

It is true node doesn't support native import syntax yet, but published package rxjs should work with CJS all time. Maybe I'm bit confused, would you mind elaborate 1. what doesn't work 2. what need to be improved to support CJS better?

@Brian-McBride
Copy link
Author

With rxjs compat, things work fine. Also, if you want to pull in large parts of the rxjs library into memory (or the whole lib) then that is fine too.

Currently, as a method to add functions to the Observable via prototype (from what I remember the first work involved in reducing RXJS bundle size) happens to offer a good way to handle lazy-loading in a server environment.

In the case of working with serverless and reducing the cold start time of functions, one method of choice has been to lazy load the operators (as well as other libraries) as needed. A Lambda might support various capabilities but only a subset used when the function is called. In that case, if you can reduce the library load for that call, cold execution time is saved. Lazy loading modules in serverless gives a middle lane in grouping similar capabilities into one lambda while also keeping the actual functional execution as tight as possible.

Although this seems like splitting hairs, over time and in areas where we want the highest performing Lambdas, we have seen immediate reduction in startup time from 500ms to 1s down to under 100ms.

This is the why. Once removing rxjs-compat, I had to start importing in the new paths. They worked, however I found I was immediately pulling in more functions than I needed. My cold start time shot up. In the serveless world our bundle size is less important about our memory and execution size. RXJS is super popular in the JS client space, it's awesome in the backend too.

My basic ask is to consider the node environment and keep offering either a prototype lazyload (as in my first code example) or in the new pathing clean pathing for require() observables without needing to import only functionality needed. With the assumption that server can have big bundle, no tree shaking will happen, and in-memory loading is the high-cost operation in a serverless environment.

@benlesh
Copy link
Member

benlesh commented Apr 26, 2018

I found I was immediately pulling in more functions than I needed

Is this problem that we only have one export site? Meaning this pulls in too much:

function foo(source) {
  const { map, filter, scan } = require('rxjs/operators');
  return source.pipe(map(fn1), filter(fn2), scan(fn3));
}

If so, it's a tall order to fix that, because we really, really do not want to introduce hundreds of import sites again. The crazy high number of import sites was a huge problem for the majority of our developers.

I'm not sure how to solve this problem for you other than one of three ways:

  1. Using a bundler or code splitting tool (Node people seem to be irrationally against this idea usually)
  2. Importing from "internal" locations in rxjs. (With the knowledge this might break on you as you update, so you'd have to update carefully)
  3. The RxJS team creating another package that exported things separately. (Unlikely, as we don't want to further complicate or confuse things, and we have limited engineering hours)

@Brian-McBride
Copy link
Author

I can align with the benefits of consolidated packages and the larger usage of rxjs in front-end frameworks that utilize splitting/bundling.

Thanks for the thoughts. If you keep this ticket open for a week or so, I'll delve into ideas and I'll continue the thread with recommendations that I found for others working in Node & serverless looking for optimal performance gains.

@jasonaden
Copy link
Collaborator

@Brian-McBride Definitely an interesting discussion, and probably not something specific to RxJS. Though RxJS might be a good use-case where it's heavily used in a bundled environment, Node environment, and serverless.

For the most part, the size/memory issue hasn't been considered much for CJS. But the serverless environment and cold startup time may force a change in how this is considered. At the same time, if everyone continues to create workarounds for Node not supporting ES6 imports, we will never get to a standardized module pattern.

Certainly worth continuing the discussion.

@DanielJoyce
Copy link

Yeah, this is a HUGE pain right. UGH

@benlesh
Copy link
Member

benlesh commented Oct 2, 2019

I think with current destructuring, require now works smoothly.

const { Observable, of, timer } = require('rxjs');
const { filter, map, mergeMap } = require('rxjs/operators');

Closing this for now.

@benlesh benlesh closed this as completed Oct 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 1, 2019
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

5 participants