Refactor tasks.reducer.js

[joshwcomeau]

When I created tasks.reducer.js, I wanted to keep a 'flat' structure; every task was given an ID of the form projectId-taskId, and it was 1 level deep:

{
  foo-start: { ... },
  foo-build: { ... },
  bar-start: { ... },
  bar-build: { ... },
}

This wound up requiring a fair amount of munging, and it felt like a bit of extra friction whenever I needed to read/write tasks.

When I tackled the dependencies reducer, I decided to experiment with a nested structure. So instead, each project has its own key:

{
  foo: {
    start: { ... },
    build: { ... },
  }
  bar: {
    start: { ... },
    build: { ... },
  }
}

This structure actually works quite a bit better, because no dynamic string creation is required.

However, now there's this really irksome inconsistency between two database-style reducers. It would be great to update tasks.reducer.js to match dependencies.reducer.js

I see a few parts to this refactor:

• Update the reducer in tasks.reducer.js to use the new format
• Update the selectors in the bottom of the file to retrieve the tasks properly. Right now they take taskId, but they'd need to now take projectId and taskName
• Change the Task type to remove taskId; that doesn't exist anymore
• Update the components that use this data. You can probably find most of them by project-searching for taskId
• Update getInitialState in redux-persistence.service.js to use the new structure

This is indeed a pretty big job, but it'll only get bigger as the app grows, so I think it'd be great to get this done sooner rather than later.

[V1shvesh]

Can I help out with this?

[joshwcomeau]

Can I help out with this?

@V1shvesh that'd be awesome!

Let me know if you have any questions along the way; I don't expect any huge complications, but this is a hot code path and it's used all over the project, so it's non-trivial.

Going to remove the "help wanted" issue, this'll be yours to tackle. No worries if you've changed your mind or don't have the time, though; just please let me know and I'll re-open it for others.

[V1shvesh]

@joshwcomeau No problem!

I'll surely help out as much as I can. Will keep you updated.

[joshwcomeau]

@V1shvesh how's this going? Not urgent at all, just checking in to see if you have any questions, or if I can do anything to help :)

[V1shvesh]

@joshwcomeau Hey there!
Well, I didn't know much about Redux and reducers, so I kinda went learning through the docs. I am trying to understand it myself.

I'll surely clear any doubts with you, would love your help.

Hope to fix this in 2-3 days.:relaxed:

[V1shvesh]

const buildUniqueTaskId = (projectId, name) => `${projectId}-${name}`;

I believe this isn't needed anymore?

[joshwcomeau]

Well, I didn't know much about Redux and reducers, so I kinda went learning through the docs. I am trying to understand it myself.

Ahh I understand. Ok! Very brave of you to try and tackle such a big refactor without prior redux knowledge, haha. Will be glad to answer any questions, although it may take me a while to respond, busy these days working to get a talk prepared.

Hope to fix this in 2-3 days.☺️

No rush :)

const buildUniqueTaskId = (projectId, name) => ${projectId}-${name};
I believe this isn't needed anymore?

Right. I'm pretty sure you won't need that anymore. The thing to check is where those unique IDs are used, and to see if a non-unique Id (like the task name, eg. start) will be sufficient. I believe it will be, but I'm not sure off the top of my head.

[V1shvesh]

busy these days working to get a talk prepared.

This sounds cool!

Also,

type State = {
  [projectId: string]: {
    [taskName: string]: Task,
  },
};

This'll be the structure of state, I suppose.

[joshwcomeau]

@V1shvesh yeah, that's right! The Task type will likely need to change a bit as well, to remove id.

[V1shvesh]

Hey @joshwcomeau

Please check my PR and let me know if I went wrong anywhere.

Apart from that, in redux-persistence.service.js,

const scrubbedTasks = Object.keys(reconstructedState.tasks).reduce(
  (acc, taskId) => {
    const task = { ...reconstructedState.tasks[taskId] };
    task.status = 'idle';
    task.processId = null;
    task.logs = [];

    return { ...acc, [taskId]: task };
  },
  {}
);

Do we want to have a nested structure in scrubbedTasks?

[joshwcomeau]

Yay, thanks @V1shvesh! Checking now.

For redux-persistence, it should match the same structure (since this structure will be used for the initial state on next load). So yeah it should be nested as well. Which does make this operation a bit trickier... hopefully not too bad though!