Add selector to get Posts EmbedVisible flags for RHS.

[sudheerDev]

Summary

Adding new selector for RHS posts embedVisible flags.

Checklist

• Ran make check-style to check for style errors (required for all pull requests)
• Added or updated unit tests (required for all new features)
• Has redux changes (please link)

[sudheerDev]

@hmhealey I have to send entire state and therefore computes with every state change. Should we have makeGetItem as memorised selector which depends on state.storage instead?.

[hmhealey]

makeGetItem doesn't work with selectors with how it's currently designed, so I don't think we can use it here, but we can still definitely have this memoized properly.

What we can do though is change what the selector takes from the store so that it only takes the things that affect it: the storage state, the current user ID for the storage key prefix, and the COLLAPSE_DISPLAY preference. We'll also want to have a function that creates a selector here because it could possibly be used in multiple places, and we want to make sure each one is memoized separately so they don't clear each other's stored results. It could look something like

export function makeGetPostsEmbedVisibleObj() {
    return createSelector(
        // This could be its own function or a selector, but that's not necessary since it's really simple
        (state) => state.storage,
        // We don't need to pass in the previewCollapsed value here since we can just get it ourselves.
        // Also, we can use getBool so that we don't need to work with a string later on
        (state) => getBool(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.COLLAPSE_DISPLAY),
        (state, posts) => posts,
        (storage, previewCollapsed, posts) => {
            const postsEmbedVisibleObj = {};

            for (const post of posts) {
                // You'll need to write something like makeGetItem that doesn't require the whole store to be passed in
                postsEmbedVisibleObj[post.id] = getItemFromStorage(storage, currentUserId, StoragePrefixes.EMBED_VISIBLE + post.id, previewCollapsed);
            }

            return postsEmbedVisibleObj;
        }
    );
}

That way, it'll only change when one of those exact 3 things changes, and not just any state change.

[sudheerDev]

I will make the changes. Thanks, this looks great 👍 .

[hmhealey]

Something important to note here is that this function returns a new object each time it's called, so it'll prevent memoization. Even if you were to do something like (state, props) => (props) here, props is likely changing each time this is called anyway, so it still wouldn't help. In the future though, you can break it up like this to have it work properly

createSelector(
    (state, {posts}) => posts,
    (state, {previewCollapsed}) => previewCollapsed,
    (posts, previewCollapsed) => {...}
);

As long as posts and previewCollapsed themselves don't change, it'll only be looking at their value to check if anything has changed instead of looking if the object containing them changed.

[sudheerDev]

Oh ya true. That's cool

[sudheerDev]

Yes made changes.

[hmhealey]

makeGetItem doesn't work with selectors with how it's currently designed, so I don't think we can use it here, but we can still definitely have this memoized properly.

What we can do though is change what the selector takes from the store so that it only takes the things that affect it: the storage state, the current user ID for the storage key prefix, and the COLLAPSE_DISPLAY preference. We'll also want to have a function that creates a selector here because it could possibly be used in multiple places, and we want to make sure each one is memoized separately so they don't clear each other's stored results. It could look something like

export function makeGetPostsEmbedVisibleObj() {
    return createSelector(
        // This could be its own function or a selector, but that's not necessary since it's really simple
        (state) => state.storage,
        // We don't need to pass in the previewCollapsed value here since we can just get it ourselves.
        // Also, we can use getBool so that we don't need to work with a string later on
        (state) => getBool(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.COLLAPSE_DISPLAY),
        (state, posts) => posts,
        (storage, previewCollapsed, posts) => {
            const postsEmbedVisibleObj = {};

            for (const post of posts) {
                // You'll need to write something like makeGetItem that doesn't require the whole store to be passed in
                postsEmbedVisibleObj[post.id] = getItemFromStorage(storage, currentUserId, StoragePrefixes.EMBED_VISIBLE + post.id, previewCollapsed);
            }

            return postsEmbedVisibleObj;
        }
    );
}

That way, it'll only change when one of those exact 3 things changes, and not just any state change.

[saturninoabril]

@sudheerDev Unit test failed. Try to run make test and see where it's failing.

[sudheerDev]

@saturninoabril ya I was working on top of a component branch which has a constant this commit needed. Anyway added that constant to this commit. Test cases should be fine now

[hmhealey]

Awesome, thanks for making that change. Looks good and fast now