Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

reverse spread of props on mapPropsOnChange #104

Merged
merged 3 commits into from
Mar 21, 2016

Conversation

hartzis
Copy link
Contributor

@hartzis hartzis commented Mar 2, 2016

fix for #102

@codecov-io
Copy link

Current coverage is 100.00%

Merging #104 into master will not affect coverage as of 34150c8

@@            master    #104   diff @@
======================================
  Files           42      42       
  Stmts          325     325       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit            325     325       
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of 34150c8

Powered by Codecov. Updated on successful CI builds.

@hartzis
Copy link
Contributor Author

hartzis commented Mar 2, 2016

CI build failed due to:

Module build failed: Error: The (relay-query) Babel 5 plugin is being run with Babel 6.

I think it is related to facebook/relay#889, and when they publish a new version it may be fixed.

@hartzis
Copy link
Contributor Author

hartzis commented Mar 7, 2016

blocked by tests failing, #106

@hartzis
Copy link
Contributor Author

hartzis commented Mar 21, 2016

@acdlite merged in master to get tests to run, and fixed the broken tests. Should be good to go now.

acdlite added a commit that referenced this pull request Mar 21, 2016
reverse spread of props on mapPropsOnChange
@acdlite acdlite merged commit e98a68f into acdlite:master Mar 21, 2016
@acdlite
Copy link
Owner

acdlite commented Mar 21, 2016

Thanks!

@istarkov
Copy link
Contributor

PS: PR omit props
IMHO omit should be removed.

I'll explain why:

One of my most used construction with mapPropsOnChange was

mapPropsOnChange(
  ['a', 'b', 'c'], 
  ({...props, a, b ,c}) => ({
    ...props,
    someResult: blabla(a,b,c)
  })
),

spread operator allows me to not lost a,b,c in the future, and reuse them again.

Now to make this work I can't use spread operator at all, because other props out of selection array i.e. ['a', 'b', 'c'] will be placed in this.computedProps so will be outdated.

The code above now looks like

mapPropsOnChange(
  ['a', 'b', 'c'], 
  ({a, b ,c}) => ({
    a,
    b,
    c,
    someResult: blabla(a,b,c)
  })
),

So every time I add property into selection array I need to add it into object to preserve.

Removing omit will help a lot, I could just remove all that ...props, and forget about property omitting logic in mapPropsOnChange

@istarkov istarkov mentioned this pull request Mar 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants