Move component props from stash to stash.component_props #626
Conversation
sambhav
changed the title
sambhav
force-pushed
the
portstash
branch
3 times, most recently
from
d4eaa02 to
9b3104d
Compare
sambhav
force-pushed
the
portstash
branch
2 times, most recently
from
772327c to
0ac7a80
Compare
| current_view => 'Node', | ||
| component_path => 'isrc/Index.js', | ||
| component_props => { | ||
| isrcs => $isrcs, |
There was a problem hiding this comment.
Ok, they're already in the stash so normally I would say not to serialize them twice, but the isrcs should be a small list.
There was a problem hiding this comment.
should I remove them from the stash earlier on then?
There was a problem hiding this comment.
Ah, perhaps. I thought other actions might be using them since they are set in _load, but it looks like this is the only action in this controller.
There was a problem hiding this comment.
Yup, which is why I didn't touch it.
| current_view => 'Node', | ||
| component_path => 'instrument/List', | ||
| component_props => { |
There was a problem hiding this comment.
It's a bit unsatisfactory to create broken commits like this - if someone bisecting lands on this commit, the page will be broken for them because the component isn't changed to understand this until b137a99 from what I can tell.
root/instrument/List.js
Outdated
| instrument_types: $ReadOnlyArray<InstrumentTypeT>, | ||
| instruments_by_type: {| | ||
| +[number]: $ReadOnlyArray<InstrumentT>, | ||
| unknown: $ReadOnlyArray<InstrumentT>, |
There was a problem hiding this comment.
This is now missing the + on all the props expect [number] (did you say some eslint rule was removing those? we should disable it)
Making PropsT an exact object type would be nice too: {| ... |}
No description provided.