0.3.0 - Export drivers to userland

[lmatteis]

I've separated the two things into separate PRs. This only takes care of putting run in userland without Unified support.

[goshacmd]

Did you think about my idea of scoping the drivers under cycleMiddleware returned from createCycleMiddleware? We could probably even get rid of makeXDriver functions, instead exposing already-made drivers, which makes sense:

const cycleMiddleware = createCycleMiddleware();
...

ACTION: cycleMiddleware.actionDriver

So that, in the end, createCycleMiddleware acts as both:

• middleware constructor
• but also: driver maker, since these are dependent on the middleware anyway

[lmatteis]

@goshakkk ok but what would you pass it to applyMiddleware(...)? cycleMiddleware.middleware? Because applyMiddleware(...) wants a function, and in your example createCycleMiddleware() seems to return an object.

[goshacmd]

@lmatteis functions can have properties in js 😈

function createCycleMiddleware() {
  const middleware = () => { ... };
  middleware.actionDriver = ...;
  return middleware;
}

const cycleMiddleware = createCycleMiddleware();
// cycleMiddleware is a fn
cycleMiddleware.actionDriver
// but it also has other fields

There's nothing dirty about doing it like that.

Redux-saga does something similar as well: https://github.com/redux-saga/redux-saga#mainjs

[lmatteis]

Hrm not really happy about func props. You can't destructor them. I like the idea of having the drivers depend on the middleware though.

[goshacmd]

Fwiw, you can:

function a() {}
a.b = 5;
const { b } = a;
console.log(b) ;
// prints 5

[lmatteis]

@goshakkk ok makes sense. made changes. one thing is that i'm still exposing factories (makeStateDriver rather than stateDriver) because of this:

  cycleMiddleware.makeStateDriver = () => {
    const isSame = {};
    const getCurrent = store.getState;
    return function stateDriver() {

Where would getCurrent go otherwise (depends on store which is instantiated after applyMiddleware is called)?

[goshacmd]

@lmatteis I think makeStateDriver and makeActionDriver could be functions inside createCycleMiddleware, but what would be exported are not the factory functions themselves, but driver instances:

function createCycleMiddleware() {
  let store;

  function makeStateDriver() { ... }

  const middleware = () => { ... };

  middleware.stateDriver = makeStateDriver();
  return middleware;
}

Alternatively, we might not even need the factory functions at all, just assign the driver instances directly:

function createCycleMiddleware() {
  let store;

  const middleware = () => { ... };

  middleware.stateDriver = () => {
    const getCurrent = store.getState;
    return xs.create...;
  };
  return middleware;
}

[lmatteis]

@goshakkk can we rely on that though? The driver will be executed by Cycle.js, hence const getCurrent = store.getState; might get executed in ways we can't predict (multiple times perhaps). Also in terms of API, most cycle drivers are factories. What if we may need to pass arguments in the future?

[lmatteis]

@goshakkk i added a 1.0.0 commit with a CHANGELOG.md. Let me know what you think.

[lmatteis]

This PR should probably update the code in example/ with 1.0.0 changes. Problem is that it I can't make it depend on 1.0.0 because it isn't published yet. Should I just import ../ in the examples rather than redux-cycles from npm?

nevermind I can use npm local dependencies: "redux-cycles": "file:../",

[lmatteis]

Another small comment (sorry for the spam): makeStateDriver() needs to be called after run, otherwise store.getState is undefined. Any ideas about this?

[goshacmd]

@lmatteis just move that line to be inside stateDriver?

[goshacmd]

Is it also worth mentioning here and in the readme the need to [npm i -s/yarn add] xstream-run?

[lmatteis]

Ok this might be ready to merge. Will wait for the green light from you guys

[goshacmd]

Nice. Can you tweak the readme with xstream-run install as well?

Another thing I am not sure about is — are we ready for 1.0? Or maybe should we still stay in the 0.x area?

[goshacmd]

Can the readme mention the need to install it as well?

[lmatteis]

Well it's a breaking change no? So we need to bump up the first number. 0.3 would mean new features and no breaking changes.

[goshacmd]

@lmatteis semver is pretty lax with 0.x's. 0.x means that a project is not yet ready for 1.0, but breaking changes can nevertheless naturally occur — because everything leading up to 1.0 is, in some way, "discovery" of the API, and use cases, and the bugs, and so on; v1.0 signifies the first "stable" / "reliable" version. So it's perfectly fine to make breaking changes and do a 0.3, 0.4, etc. while we're making and testing decisions.

I'd cut 1.0 when we have more people using redux-cycles in production and sharing their feedback, so we will be in position to say 1.0 is truly battle-tested.

More on semver: http://semver.org/#how-should-i-deal-with-revisions-in-the-0yz-initial-development-phase this obviously should be taken as a broad guideline, rather than a strict rule, but I do think this makes sense.

[lmatteis]

Right but most users that do npm i -s redux-cycles will have ^0.2.3 in their package.json. Meaning that if we release 0.3.0 instead of 1.0.0 it will break their code. I guess we can go for 0.3.0 since we're not widely used. But the API and code footprint is so small that it's hard to think we'd need to change the API sometimes soon - hence the fast bump to 1.0.0.

[nickbalestra]

Nice.

[lmatteis]

Jeez I didn't know there was so much disagreement with semver: dominictarr/semver-ftw#2 (comment)

[lmatteis]

Hold on, will npm not update ^0.2.3 to 0.3.0? I can't find the right docs, but if ^0.2.3 is essentially 0.2.x (because of the 0) then we can totally go for 0.3.0 instead of 1.0.0. I just don't want to break changes to users using ^0.2.3

EDIT: yup i tested it so npm is smart enough to interpret the leading 0. Ok se we can go with 0.3.0

[lmatteis]

Ok I think I took care of all comments. Waiting for your thumbsup @goshakkk

[goshacmd]

hehe, nice one 👍

[goshacmd]

I like it