React router #343
|
|
@ -8,8 +8,8 @@ const mapStateToProps = ({ show }) => {
|
|||
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
|
||||
status: show.displayAsset.status,
|
||||
};
|
||||
// select asset info
|
||||
const existingRequest = show.assetRequests[show.request.id];
|
||||
|
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
|
||||
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`;
|
||||
|
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
|
||||
const previousRequest = show.assetRequests[show.request.id];
|
||||
|
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
|
||||
const assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`;
|
||||
|
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
|
||||
const asset = show.assetList[assetKey];
|
||||
if (asset) {
|
||||
props['asset'] = asset;
|
||||
|
|
|
|||
|
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
Why do you do Why do you do `const that = this`?
I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
I had a misunderstanding of how the I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
|
||||
|
|
@ -4,8 +4,8 @@ import View from './view';
|
|||
const mapStateToProps = ({ show }) => {
|
||||
let props = {};
|
||||
// select title
|
||||
const existingRequest = show.assetRequests[show.request.id];
|
||||
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`;
|
||||
const previousRequest = show.assetRequests[show.request.id];
|
||||
const assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`;
|
||||
const asset = show.assetList[assetKey];
|
||||
if (asset) {
|
||||
props['title'] = asset.claimData.title;
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ import View from './view';
|
|||
|
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
|
||||
const mapStateToProps = ({ show }) => {
|
||||
let props = {};
|
||||
// select name and claim id
|
||||
const existingRequest = show.assetRequests[show.request.id];
|
||||
|
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
|
||||
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`;
|
||||
|
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
|
||||
const previousRequest = show.assetRequests[show.request.id];
|
||||
|
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
|
||||
const assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`;
|
||||
|
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
|
||||
const asset = show.assetList[assetKey];
|
||||
if (asset) {
|
||||
props['name'] = asset.name;
|
||||
|
|
|
|||
|
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
You can use destructuring twice to avoid all the repeated
Then you can just use You can use destructuring twice to avoid all the repeated `this.props.claimData`
`const { claimData: { name, claimId... } } = this.props`
Then you can just use `name={name}`
|
||||
|
|
@ -3,17 +3,16 @@ import { updateChannelClaimsAsync } from 'actions/show';
|
|||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
import View from './view';
|
||||
|
||||
const mapStateToProps = ({ show }) => {
|
||||
let props = {};
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
// select channel key
|
||||
const request = show.channelRequests[show.request.id];
|
||||
const channelKey = `c#${request.name}#${request.longId}`;
|
||||
props['channelKey'] = channelKey;
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
// select channel claims
|
||||
const channel = show.channelList[channelKey];
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
if (channel) {
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
props['channel'] = channel;
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
const channel = show.channelList[channelKey] || null;
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
// return props
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
return {
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
channelKey,
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
channel,
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
};
|
||||
return props;
|
||||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
};
|
||||
|
||||
const mapDispatchToProps = dispatch => {
|
||||
|
|
|
|||
|
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
Will these nested values always exist? Will these nested values always exist?
They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking? They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
I was just wondering if there would ever be a case when Or if any of those children would be undefined which would throw an error I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`.
Or if any of those children would be undefined which would throw an error
|
||||
|
|
@ -3,25 +3,30 @@ import View from './view';
|
|||
import { newAssetRequest, showNewAsset } from 'actions/show';
|
||||
|
||||
const mapStateToProps = ({ show }) => {
|
||||
let props = {};
|
||||
props['requestType'] = show.request.type;
|
||||
props['requestId'] = show.request.id;
|
||||
props['requestName'] = show.request.data.name;
|
||||
props['requestModifier'] = show.request.data.modifier;
|
||||
props['requestExtension'] = show.request.data.extension;
|
||||
// select request info
|
||||
const requestType = show.request.type;
|
||||
const requestId = show.request.id;
|
||||
const requestName = show.request.data.name;
|
||||
const requestModifier = show.request.data.modifier;
|
||||
const requestExtension = show.request.data.extension;
|
||||
// select request
|
||||
const existingRequest = show.assetRequests[show.request.id];
|
||||
if (existingRequest) {
|
||||
props['existingRequest'] = existingRequest;
|
||||
// select asset info
|
||||
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`; // note: just store this in the request
|
||||
const asset = show.assetList[assetKey];
|
||||
if (asset) {
|
||||
console.log('existing asset found', asset);
|
||||
props['asset'] = asset;
|
||||
};
|
||||
const previousRequest = show.assetRequests[show.request.id] || null;
|
||||
// select asset infogit
|
||||
let asset;
|
||||
if (previousRequest) {
|
||||
const assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`; // note: just store this in the request
|
||||
asset = show.assetList[assetKey] || null;
|
||||
};
|
||||
// return props
|
||||
return {
|
||||
requestType,
|
||||
requestId,
|
||||
requestName,
|
||||
requestModifier,
|
||||
requestExtension,
|
||||
previousRequest,
|
||||
asset,
|
||||
};
|
||||
return props;
|
||||
};
|
||||
|
||||
const mapDispatchToProps = dispatch => {
|
||||
|
|
|
|||
|
|
@ -11,23 +11,23 @@ function requestIsAnAssetRequest ({ requestType }) {
|
|||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
|
||||
class ShowAsset extends React.Component {
|
||||
componentDidMount () {
|
||||
const { asset, existingRequest, requestId, requestName, requestModifier } = this.props;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
if (!existingRequest) { // case: the asset request does not exist
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
const { asset, previousRequest, requestId, requestName, requestModifier } = this.props;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
if (!previousRequest) { // case: the asset request does not exist
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
return this.props.onNewRequest(requestId, requestName, requestModifier);
|
||||
};
|
||||
if (!asset) { // case: the asset request does not exist
|
||||
const { name, claimId } = existingRequest;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
const { name, claimId } = previousRequest;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
return this.props.onShowNewAsset(name, claimId);
|
||||
};
|
||||
}
|
||||
componentWillReceiveProps (nextProps) {
|
||||
if (requestIsAnAssetRequest(nextProps)) {
|
||||
const { asset, existingRequest, requestId, requestName, requestModifier } = nextProps;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
if (!existingRequest) {
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
const { asset, previousRequest, requestId, requestName, requestModifier } = nextProps;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
if (!previousRequest) {
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
return this.props.onNewRequest(requestId, requestName, requestModifier);
|
||||
};
|
||||
if (!asset) {
|
||||
const { name, claimId } = existingRequest;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
const { name, claimId } = previousRequest;
|
||||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
return this.props.onShowNewAsset(name, claimId);
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
I think you are still creating more work than necessary with this. In my opinion I also think I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request.
I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
Ok, I think I'm getting closer. I was able to do away with Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
|
||||
|
|
@ -4,24 +4,27 @@ import { newChannelRequest, showNewChannel } from 'actions/show';
|
|||
import View from './view';
|
||||
|
||||
const mapStateToProps = ({ show }) => {
|
||||
let props = {};
|
||||
props['requestId'] = show.request.id;
|
||||
props['requestType'] = show.request.type;
|
||||
props['requestChannelName'] = show.request.data.name;
|
||||
props['requestChannelId'] = show.request.data.id;
|
||||
// select request info
|
||||
const requestId = show.request.id;
|
||||
const requestType = show.request.type;
|
||||
const requestChannelName = show.request.data.name;
|
||||
const requestChannelId = show.request.data.id;
|
||||
// select request
|
||||
const existingRequest = show.channelRequests[show.request.id];
|
||||
if (existingRequest) {
|
||||
props['existingRequest'] = existingRequest;
|
||||
console.log('existing channel request found', existingRequest);
|
||||
// select channel info
|
||||
const channelKey = `c#${existingRequest.name}#${existingRequest.longId}`;
|
||||
const channel = show.channelList[channelKey];
|
||||
if (channel) {
|
||||
props['channel'] = channel;
|
||||
};
|
||||
const previousRequest = show.channelRequests[show.request.id] || null;
|
||||
// select channel info
|
||||
let channel;
|
||||
if (previousRequest) {
|
||||
const channelKey = `c#${previousRequest.name}#${previousRequest.longId}`;
|
||||
channel = show.channelList[channelKey] || null;
|
||||
}
|
||||
return props;
|
||||
return {
|
||||
requestId,
|
||||
requestType,
|
||||
requestChannelName,
|
||||
requestChannelId,
|
||||
previousRequest,
|
||||
channel,
|
||||
};
|
||||
};
|
||||
|
||||
const mapDispatchToProps = dispatch => {
|
||||
|
|
|
|||
|
|
@ -11,23 +11,23 @@ function requestIsAChannelRequest ({ requestType }) {
|
|||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
|
||||
class ShowChannel extends React.Component {
|
||||
componentDidMount () {
|
||||
const { existingRequest, channel, requestId, requestChannelName, requestChannelId } = this.props;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
if (!existingRequest) {
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
const { previousRequest, channel, requestId, requestChannelName, requestChannelId } = this.props;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
if (!previousRequest) {
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
return this.props.onNewChannelRequest(requestId, requestChannelName, requestChannelId);
|
||||
}
|
||||
if (!channel) {
|
||||
const { name, shortId, longId } = existingRequest;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
const { name, shortId, longId } = previousRequest;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
return this.props.onShowNewChannel(name, shortId, longId);
|
||||
}
|
||||
}
|
||||
componentWillReceiveProps (nextProps) {
|
||||
if (requestIsAChannelRequest(nextProps)) {
|
||||
const { existingRequest, channel, requestId, requestChannelName, requestChannelId } = nextProps;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
if (!existingRequest) {
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
const { previousRequest, channel, requestId, requestChannelName, requestChannelId } = nextProps;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
if (!previousRequest) {
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
return this.props.onNewChannelRequest(requestId, requestChannelName, requestChannelId);
|
||||
}
|
||||
if (!channel) {
|
||||
const { name, shortId, longId } = existingRequest;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
const { name, shortId, longId } = previousRequest;
|
||||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
return this.props.onShowNewChannel(name, shortId, longId);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
Same comments above about Same comments above about `previousRequest`. I think a more understandable approach would just be:
```
if (!channel) this.props.onNewChannelRequest(...)
```
See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done. See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
|
||||
Why do you do
const that = this?I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same
<AssetDisplay />it will make these requests again, even if you just made them a second ago