[Sources] Use Immutable records #5389
Conversation
src/reducers/sources.js
Outdated
|
|
||
| return sources.find(source => source.get("url") === url); | ||
| return sources.find(source => { | ||
| return source.get("url") === url; |
There was a problem hiding this comment.
this could be source.url == url
There was a problem hiding this comment.
I've tried but it triggers some errors and tests failed
There was a problem hiding this comment.
Do you happen to know which? Travis says everything is all good :)
Also, can we do:
return sources.find(source => source.get("url") === url);
|
I get these errors If I put
|
|
Wow, you're right; |
|
I've done some logs and I discovered that even the source in the function getPrettySource with
|
src/reducers/sources.js
Outdated
| const updatedSource = existingSource.merge(source); | ||
| return state.setIn(["sources", source.id], updatedSource); | ||
| } | ||
| return state.mergeIn(["sources", source.id], new SourceRecordClass(source)); |
There was a problem hiding this comment.
this converts the new Record into a Map, see this immutable bug. This causes the failures to retrieve attributes without using get().
There was a problem hiding this comment.
Thanks! is there a good work around?
There was a problem hiding this comment.
hey @jasonLaster... I changed state.mergeIn() in L197 to:
state.setIn(["sources", source.id], new SourceRecordClass(source);
and every source.get("x") with source.x and all tests now pass! I think @DavideDeCenzo may try to finalize it
There was a problem hiding this comment.
oh thanks. that's promising. Re-opening
|
I think this PR has kinda gone out of scope. @DavideDeCenzo how do you feel about picking up something else while we try and figureout immutable.js! |
|
Thanks @giatorta Now It works! |
|
@jasonLaster Now it should be better , the tests are passed also |
jasonLaster
force-pushed
the
Immutable-records
branch
from
ece1710 to
c335f2b
Compare
darkwing
force-pushed
the
Immutable-records
branch
from
c335f2b to
5c256fa
Compare
|
We're in! Great job @DavideDeCenzo |
4 participants
Associated Issue: #4935
Here's the Pull Request Doc
https://devtools-html.github.io/debugger.html/CONTRIBUTING.html#pull-requests
Here's the Debugger's Testing doc
https://docs.google.com/document/d/1oBMRxV8A2ag2t22YsQOxTdEv0mXKzIg0tggJjRkU1S0/edit#.
Feel free to improve it!