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

Monorepo and Splitting Out To Different Packages #6786

Open
benlesh opened this issue Jan 28, 2022 · 4 comments
Open

Monorepo and Splitting Out To Different Packages #6786

benlesh opened this issue Jan 28, 2022 · 4 comments
Labels
8.x Issues and PRs for version 8.x AGENDA ITEM Flagged for discussion at core team meetings

Comments

@benlesh
Copy link
Member

benlesh commented Jan 28, 2022

Goals

  1. We've wanted to move to a monorepo for quite some time. This is so we can move the docs app out of the project for core components, and possible move things into our monorepo to ensure compatability — for example chrome tools or the eslint plugin.
  2. We've discussed many times splitting Observable off to be a standalone package.
  3. We may want to consider splitting rxjs/testing, rxjs/ajax, etc, off into their own packages as well.
  4. We want to do this in the slowest, most easy to migrate to way possible.

Monorepo and multiple packages

  • I think we should try to button up new feature work and the like for version 7 (which we are very close to doing, if we're not done already) and move to work on version 8 in earnest.
  • In version 8, we'll keep the same exports (with regards to rxjs, rxjs/operators, rxjs/ajax, et al) but split all of those off into their own packages under @rxjs/observable, @rxjs/operators, @rxjs/ajax, etc. We'll want to bikeshed ideas like @rxjs/subjects or @rxjs/multicasting etc. There's a lot of ground to cover there, and I don't want to get bogged down on that in this discussion.
  • In version 8 we're going to reexport from the new packages. For example, rxjs will export { Observable } from '@rxjs/observable'; etc.

Standalone Observable

  • Moving away from lift: If we want a standalone Observable, we should move away from lift, because it requires too much knowledge of the inner working of RxJS in order to create custom operators that leverage it properly.
  • We can start supporting Observable that doesn't have lift as a non-breaking change anytime. This is completely contained within our operate utility function now. We may even start supporting this in v7, as it should be completely innocuous. It's just a different call pattern. I actually have some work done around this, and everything looks solid.
  • I think we're going to need to leave pipe on our standalone observable package, as it's central to our particular design for how to use RxJS. Even though pipe might not be immediately useful to users of Observable that don't care about operators. (libraries that export Observable, etc).

Long Term:

  1. We wouldn't want to force people to move over to @rxjs/observable et al until version 9. That should be a long ways off.
  2. Operator creation needs to be consumerized. We have our out internal way of creating operators, and we need to come up with a way of making that consumable and understandable to the masses. It has some advantages, like properly unsubscribing from firehose inner observables. (Currently it's impossible to do this properly with plan observer implementations) (A token/signal approach would help this)
  3. Considerations around using abort signals, etc.
@benlesh benlesh added 7.x Issues and PRs for version 7.x 8.x Issues and PRs for version 8.x AGENDA ITEM Flagged for discussion at core team meetings and removed 7.x Issues and PRs for version 7.x labels Jan 29, 2022
benlesh added a commit to benlesh/rxjs that referenced this issue Feb 8, 2022
* Adds a code path for operating on a Subscribable without lift.
* Loosens the type of `OperatorFunction` and therefore `MonoTypeOperatorFunction` to go from `Subscribable<In>` to `Subscribable<Out>`.

This is done in preparation for splitting `Observable` into its own package, where `lift` doesn't really need to exist in the public API.

Related ReactiveX#6803 ReactiveX#6786
@benlesh benlesh mentioned this issue Mar 4, 2022
40 tasks
@benlesh
Copy link
Member Author

benlesh commented Mar 9, 2022

Core Team:

  1. Monorepo - 👍
  2. Creating "mini-packages" like @rxjs/observable, et al - 👍
    a. Level of granularity? What do we do with things like mergeInternals? What about operators derived from other operators (mergeMap and concatMap for example)? ❓

@PowerKiKi
Copy link

I've posted this on #6367, but I feel it should also be seen here, sorry for duplicate:

Wouldn't publishing multiple packages for what is essentially the same code actually increase risk of non tree shakable use-case and thus increase bundle size ?

As seen with lodash, if one of my dependency decides to depends on the future @rxjs/observable, but my app depends on the current rxjs, then all the code from @rxjs/observable will essentially be bundled twice, won't it ?

And of course it would be much worse if we actually publish a package for each operator. Then a single operator could be bundled three times if my deps happened to depend on rxjs, @rxjs/operators and say @rxjs/operators-map.

I could also imagine compatibility issues if one of my dependency depends on an old version of @rxjs/observable and my app pass one of those object to a new, incompatible version of an operator coming from the latest version of rxjs. This kind of issue already happened with the graphql package where they had to add manual check in their code to throw error if it happens.

That seems a lot of tricky tradeoffs to attempt to solve the issue of bundle size that is already properly solved by tree shaking.

Is that really worth it ? 🤔

@PowerKiKi
Copy link

@benlesh would you please share the rxjs team opinions about the risks mentioned in my previous post?

How could we avoid fragmenting the ecosystem and actually increase the bundle size if things are available from multiple packages at once ?

How can we solve incompatibilities across future version of rxjs, that are transient dependencies, and that will still have to collaborate with each other?

Also what do you think about the fact that lodash seems to be going into the opposite direction, single ESM tree-shakable package, lodash/lodash#5107 (comment) ?

@jollytoad
Copy link

(Sorry originally posted this in the roadmap issue, before realising this issue is probably more appropriate)

I just wanted to contribute to the discussion of packaging ... I've been successfully using rxjs 7.5.x in Deno via esm.sh, but the way things are imported isn't ideal, ie. everything is import { ... } from 'rxjs' (where rxjs is actually mapped to an esm.sh url in an import map) ... treeshaking isn't an option in this situation, so the whole of rxjs ends up being loaded whether used or not.

I'm not bothered really whether ops are individually packaged or not, but would prefer a situation where they are at least exposed publicly as individual modules, so I can do something like: import { concatMap } from 'rxjs/op/concatMap'; and in turn that imports only on what it needs (not the whole of rxjs) ... I know rxjs modules already exist like this, but they are all under an internal module structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.x Issues and PRs for version 8.x AGENDA ITEM Flagged for discussion at core team meetings
Projects
None yet
Development

No branches or pull requests

3 participants