MichaelContento/redux-storage

Fork 0

Add meta.origin to our saveAction if NODE_ENV != production #102

Merged

michaelcontento merged 1 commit from saveAction-meta-origin into master

2016-02-29 09:38:14 +00:00

michaelcontento commented

2016-02-25 13:54:39 +00:00

(Migrated from github.com)

Fixes #100

michaelcontento commented

2016-02-26 16:59:34 +00:00

(Migrated from github.com)

@mathieudutour Do you have a opinion about this? You're quite active on this project and I'd love to hear your feedback 😄

@mathieudutour Do you have a opinion about this? You're quite active on this project and I'd love to hear your feedback :smile:

mathieudutour commented

2016-02-26 17:21:44 +00:00

(Migrated from github.com)

Yeah it might be useful. This is a breaking change though. And you need to add

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

to the package.json

Yeah it might be useful. This is a breaking change though. And you need to add ``` "browserify": { "transform": [ "envify" ] }, ``` to the package.json

michaelcontento commented

2016-02-29 08:51:13 +00:00

(Migrated from github.com)

@mathieudutour I've added envify (the faster variant named loose-envify - the same as redux is using).

But why should this be a breaking change? It does not manipulate the actual payload of the save action. I though of this as a minor release

@mathieudutour I've added envify (the faster variant named [loose-envify](https://github.com/zertosh/loose-envify) - the same as [redux](https://github.com/reactjs/redux/blob/master/package.json#L116-L120) is using). But why should this be a breaking change? It does not manipulate the actual payload of the save action. I though of this as a minor release

mathieudutour commented

2016-02-29 09:10:17 +00:00

(Migrated from github.com)

Because of someone is already using the origin meta, it breaks.

For it to be non-breaking, it needs to be opt-in

On 29 Feb 2016, at 08:51, Michael Contento notifications@github.com wrote:

@mathieudutour https://github.com/mathieudutour I've added envify (the
faster variant named loose-envify https://github.com/zertosh/loose-envify

the same as redux
https://github.com/reactjs/redux/blob/master/package.json#L116-L120 is
using).

But why should this be a breaking change? It does not manipulate the actual
payload of the save action. I though of this as a minor release

—
Reply to this email directly or view it on GitHub
https://github.com/michaelcontento/redux-storage/pull/102#issuecomment-190107501
.

Because of someone is already using the origin meta, it breaks. For it to be non-breaking, it needs to be opt-in On 29 Feb 2016, at 08:51, Michael Contento notifications@github.com wrote: @mathieudutour https://github.com/mathieudutour I've added envify (the faster variant named loose-envify https://github.com/zertosh/loose-envify - the same as redux https://github.com/reactjs/redux/blob/master/package.json#L116-L120 is using). But why should this be a breaking change? It does not manipulate the actual payload of the save action. I though of this as a minor release — Reply to this email directly or view it on GitHub https://github.com/michaelcontento/redux-storage/pull/102#issuecomment-190107501 .

michaelcontento commented

2016-02-29 09:16:22 +00:00

(Migrated from github.com)

Oh! You mean if someone hijacks and mutates the saveAction while it is been processed? Good point! Although this sounds more like a redux anti-pattern to me 😄

But as this is a good point, it will be part of the upcoming 4.0.0 together with the fromJS changes 👍

Oh! You mean if someone hijacks and mutates the saveAction while it is been processed? Good point! Although this sounds more like a redux anti-pattern to me :smile: But as this is a good point, it will be part of the upcoming 4.0.0 together with the `fromJS` changes :+1:

michaelcontento commented

2016-02-29 09:18:45 +00:00

(Migrated from github.com)

NOTE: Branch rebased as browserify-envify is already required in the current master

**NOTE**: Branch rebased as `browserify-envify` is already required in the current master

mathieudutour commented

2016-02-29 09:19:27 +00:00

(Migrated from github.com)

No not necessary. I have some middlewares checking for the existence of a
meta key to trigger a side effect. Someone could have one of those
listening to the origin meta key

On 29 Feb 2016, at 09:16, Michael Contento notifications@github.com wrote:

Oh! You mean if someone hijacks and mutates the saveAction while it is been
processed? Good point! Although this sounds more like a redux anti-pattern
to me [image: 😄]

But as this is a good point, it will be part of the upcoming 4.0.0 together
with the fromJS changes [image: 👍]

—
Reply to this email directly or view it on GitHub
https://github.com/michaelcontento/redux-storage/pull/102#issuecomment-190115959
.

No not necessary. I have some middlewares checking for the existence of a meta key to trigger a side effect. Someone could have one of those listening to the origin meta key On 29 Feb 2016, at 09:16, Michael Contento notifications@github.com wrote: Oh! You mean if someone hijacks and mutates the saveAction while it is been processed? Good point! Although this sounds more like a redux anti-pattern to me [image: :smile:] But as this is a good point, it will be part of the upcoming 4.0.0 together with the fromJS changes [image: :+1:] — Reply to this email directly or view it on GitHub https://github.com/michaelcontento/redux-storage/pull/102#issuecomment-190115959 .