Investigate dropping all external dependencies

[markerikson]

We currently have two external dependencies: https://github.com/zertosh/loose-envify and https://github.com/benlesh/symbol-observable .

loose-envify appears to be used in package.json, but only as some kind of polyfill / build plugin for Browserify:

redux/package.json

Lines 102 to 106 in 85024d4

	"browserify": {
	"transform": [
	"loose-envify"
	]
	},

symbol-observable is used in createStore as a polyfill for Symbol.observable, to allow compatibility for treating the Redux store as an observable:

redux/src/createStore.ts

Line 1 in 85024d4

import $$observable from 'symbol-observable'

redux/src/createStore.ts

Lines 331 to 333 in 85024d4

	[$$observable]() {
	return this
	}

I would like to strongly consider dropping both of those dependencies if at all possible.

I question whether we need to have any specific support for Browserify in here. If there's a build step that needs to be done, let the end users take care of it. No reason to have an installation dep just for that.

symbol-observable boils down to this 15-line file for the polyfill plus another 15 lines for figuring out what the global object is.

@davidkpiano suggests rolling our own symbol-observable if really needed.

Finally, we are currently talking about adding ts-toolbelt as a dependency for its compose function. I would like us to seriously investigate whatever other alternatives are out there to adding a dep, even if it's one that ultimately compiles away.

[mpeyper]

Could symbol-observable be added to the store using an enhancer rather than adding it by default?

import $$observable from 'symbol-observable'

export const asObservable = (createStore) => (...args) => {
  const store = createStore(...args)
  store.observable = function() {
    const outerSubscribe = store.subscribe
    return {
      subscribe(observer) {
        if (typeof observer !== 'object' || observer === null) {
          throw new TypeError('Expected the observer to be an object.')
        }

        function observeState() {
          const observerAsObserver = observer
          if (observerAsObserver.next) {
            observerAsObserver.next(getState())
          }
        }

        observeState()
        const unsubscribe = outerSubscribe(observeState)
        return { unsubscribe }
      }

      [$$observable]() {
        return this // not sure if this should be `this` or `store`
      }
    }
  }
  return store
}

const store = createStore(reducer, asObservable)

const store = createStore(reducer, compose(asObservable, applyMiddleware(epicMiddleware)))

The enhancer could be a separate package, included in redux-observable or as a distinct import (e.g. import { asObservable } from 'redux/observable')

This was just a thought when I saw your tweet. It may not be practical or even work at all.

[markerikson]

"Could", yes, because a store enhancer returns a new store object and thus gets to completely override any part of the store API (or even add new things that aren't part of the API).

I'm less concerned about the bit of code, and more just wanting to figure out if we can remove the need to depend on symbol-observable.

Also, from what I can read, Symbol.observable is meant to be a universal cross-lib indicator that a thing can be treated as an observable, and is part of the notional ES Observables spec rather than the `redux-observable lib.

[mpeyper]

Yeah, I guess I was suggesting to drop it because it could be added in userland by whoever actually had that requirement.

I only suggested redux-observable as I thought it might make a nice home for a redux to observable bridge.

[markerikson]

Some RxJS folks point out that RxJS itself no longer has symbol-observable as a dependency:

https://twitter.com/robwormald/status/1170420603481837568

https://github.com/ReactiveX/rxjs/blob/fc3d4264395d88887cae1df2de1b931964f3e684/src/internal/symbol/observable.ts

[timdorr]

How do we polyfill the Symbol correctly without the dep? As long as that's possible (I don't see why it wouldn't be), then I'm all for removing the dep.

I'm also good with dropping loose-envify. I never quite understood why it was necessary, other than bad architecture in Browserify. That's not really our responsibility to fix.

[markerikson]

Per the linked RxJS file, the polyfill boils down to:

export const observable = (() => typeof Symbol === 'function' && Symbol.observable || '@@observable')();

[Andarist]

Out of curiosity - what's the argument behind dropping symbol-observable? Not that I care that much, not using it at all, but the package is really small and self-contained. What is the benefit of inlining its code (and thus spreading duplication in the ecosystem)?

I once had to debug a really nasty timing issue around Symbol polyfilling. It wouldn't probably be completely prevented by having a single package like symbol-observable, but the risk would be radically minimized.

I believe I was debugging an issue on IE11 when our app has stopped to work. And I've managed to investigate the bug into react-dom and react packages not sharing their symbols (like REACT_ELEMENT_TYPE) through module system, but rather through a global scope (which is basically what you are going to do by dropping symbol-observable package).

My problem was that a Symbol got polyfilled in between react & react-dom loads, so first load (let's say react) did not have a Symbol available globally yet and it has created fallback number types, but latter react-dom had it available so it has used Symbol.for to create those types. This lead to a mismatch of types - they have to be the same for react & react-dom packages working OK together.

Loading polyfills not at the top of our app was a bug in itself, but is has slipped because the first react-related load was deeply hidden in an utils file referenced by some other code which we have wanted to execute first - as soon as possible, even before polyfills (error reporting related module).

So long story short - if react & react-dom would use a shared package I (or somebody else) wouldn't have to dig into this edge case problem.

[timdorr]

I mean, if you redefine what window.Symbol is midway through execution, there's not really much that can be done to fix that. 🤷‍♂

Anyways, I think the byte overhead of symbol-observable vs this small polyfill makes it come out in the wash anyways. It's not really a practical issue.

[Andarist]

I agree that this was a strange issue on our side, and not something that happens daily - but a centralized package for those React types would actually save hours of debugging in this case. So it's not really that "not much can be done to fix that". I understand though that this is an edge case and not a strong argument in favor of this - just something I've wanted to mention.

As to the byte overhead - it's 90 bytes difference (minified+gzipped on both). So not really something I would personally call a significant gain.

Don't get me wrong - I'm here just to discuss as I'm interested in general reasoning/arguments behind changes like this. Lately, I've noticed people wanting to get rid of all dependencies and I just wonder why that is. Personally I don't like bloated dependencies myself either, but inlining really small helpers just for sake of removing a dep is somewhat against composition for me.

[markerikson]

I have commented several times that the JS ecosystem would be well served by trying to do some consolidation and reduce tiny dependencies. There's no reason that a CRA+TS app should end up with 1500 deps downloaded.

In Redux's case specifically, there's no need to add a package dependency just to get this tiny polyfill. I would love to see v5.0 have zero additional dependencies after installation.

[jednano]

I think the modern (2019) approach is to not include polyfills in libraries, but do communicate which polyfills people need to install in order to support certain browsers.

ky is a good example of this:

Ky targets modern browsers and Deno. For older browsers, you will need to transpile and use a fetch polyfill. For Node.js, check out Got. For isomorphic needs (like SSR), check out ky-universal.

[markerikson]

Sounds good to me.

[markerikson]

Still need to get rid of loose-envify.

[timdorr]

Yep. That was GitHub auto-closing it.

[timdorr]

Removed in 17f0ea0.