Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement] Add wrapper for subscribing to multiple Onyx keys in non-React files #108

Open
marcaaron opened this issue Oct 27, 2021 · 0 comments

Comments

@marcaaron
Copy link
Contributor

Problem

  • Subscribing to keys in non React component requires a bit of boilerplate code e.g. take these examples in the libs/actions/Report.js file
let currentUserEmail;
let currentUserAccountID;
Onyx.connect({
    key: ONYXKEYS.SESSION,
    callback: (val) => {
        // When signed out, val is undefined
        if (val) {
            currentUserEmail = val.email;
            currentUserAccountID = val.accountID;
        }
    },
});

let lastViewedReportID;
Onyx.connect({
    key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
    callback: val => lastViewedReportID = val ? Number(val) : null,
});

let myPersonalDetails;
Onyx.connect({
    key: ONYXKEYS.MY_PERSONAL_DETAILS,
    callback: val => myPersonalDetails = val,
});

These are all largely doing the same work:

  • Subscribing to a key
  • Storing a copy of the value
  • Updating that value

Why is this important?

As the use of Onyx grows we will have many patterns like this that lead to redundancy in code and should consider offering more intuitive ways to work with the code.

There are also downsides to the current style (although they have not caused problems as far as I know):

  • local variables are not "read-only" and can be modified directly
  • it's often hard to tell whether something comes from Onyx or not

Solution

Just one idea. But we could make it possible to subscribe to multiple keys by using Onyx.connectMultiple([...keys]) in a zero config way.

Here's how the above would be refactored:

const onyxValues = Onyx.connectMultiple([
    ONYXKEYS.SESSION,
    ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
    ONYXKEYS.MY_PERSONAL_DETAILS
]);

The call will create a closure and onyxValues will return an non-writable object reference that can be updated with new values. This way it will be clear when we are using a live value from Onyx vs. some other local variable (and we won't need to set up so many).

In practice, we'd just need to update some variable usages like this...

    if (reportID === onyxValues.currentlyViewedReportID && Visibility.isVisible()) {
        Log.info('[LOCAL_NOTIFICATION] No notification because it was a comment for the current report');
        return;
    }

If we needed more flexibility (like breaking up variables into multiple) we could add any transformation logic in a callback similar to how withOnyx() might massage values in a render() method.

const transformValues = ({session, currentlyViewedReportID, ...rest}) => ({
    ...rest,
    lastViewedReportID: curentlyViewedReportID ? Number(currentlyViewedReportID) : null,
    currentUserEmail: session.email,
    currentUserAccountID: session.accountID,
});

const onyxValues = Onyx.connectMultiple([
    ONYXKEYS.SESSION,
    ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
    ONYXKEYS.MY_PERSONAL_DETAILS,
], transformValues);

Maybe there are other ideas how to clean this up or it wouldn't be a priority to do right now, but wanted to float the idea out there.

@arosiclair arosiclair assigned arosiclair and unassigned arosiclair Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants