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

setSelf() in atom effect produce releaseNodesNowOnCurrentTree error #1582

Closed
domis77 opened this issue Jan 31, 2022 · 3 comments
Closed

setSelf() in atom effect produce releaseNodesNowOnCurrentTree error #1582

domis77 opened this issue Jan 31, 2022 · 3 comments
Assignees

Comments

@domis77
Copy link

domis77 commented Jan 31, 2022

I'm using effect in my atom, which causing error during changing state.

type blockchainStateType = {
    name: string,
    blockchain?: IAVAILABLE_BLOCKCHAINS | undefined
}

export const blockchainState = atom<blockchainStateType>({
    key: 'currentBlockchainName',
    default: {
        name: localStorage.getItem("currentBlockchainName") || DEFAULT_BLOCKCHAIN,
        blockchain: getCurrentBlockchain()
    },
    effects: [
        ({onSet, setSelf}) => {
            onSet(newValue => {
                localStorage.setItem("currentBlockchainName", newValue.name);
                const currentBlockchain = getCurrentBlockchain();
                setSelf({
                    name: newValue.name,
                    blockchain: currentBlockchain
                })
            })
        }
    ]
})

Could someone explain me what am I doing wrong, or what could causing the issue?

@drarmstr drarmstr added pending Pending more information or user action and removed pending Pending more information or user action labels Jan 31, 2022
@drarmstr
Copy link
Contributor

It appears this console message is a result of the new rendering that keeps Recoil and React state in sync when done from the same batch. The unit tests for this pattern are currently passing, though they do have the error message. Will investigate further.

@drarmstr drarmstr self-assigned this Feb 3, 2022
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 3, 2022
Summary: Add a simple test for the use-case of always transforming an atom value with an effect using `setSelf()` in `onSet()` based on concern in facebookexperimental#1582.

Reviewed By: habond

Differential Revision: D33908670

fbshipit-source-id: f9da1c1c19fb45c1c439bc0427d9e95d6ae031b3
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 3, 2022
Summary: Add a simple test for the use-case of always transforming an atom value with an effect using `setSelf()` in `onSet()` based on concern in facebookexperimental#1582.

Differential Revision: D33908670

fbshipit-source-id: e827578d0021f2ae10b0f49ce1a482aebcdba17e
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 3, 2022
Summary:
Avoid releasing retainables at the end of the batch if there were writes while processing them.  This could happen, for example, if there are effects which monitor changes and then also write state.

This error is currently superfulous for the open source release where garbage collection is enabled but not used.

Addresses facebookexperimental#1582

Differential Revision: D33965759

fbshipit-source-id: 29d46a3f95c52fabf38460241b1e6e4168b44b5c
@drarmstr drarmstr linked a pull request Feb 3, 2022 that will close this issue
@drarmstr
Copy link
Contributor

drarmstr commented Feb 3, 2022

@domis77 - Looks like the error message should be spurious and can be disregarded.

@domis77
Copy link
Author

domis77 commented Feb 3, 2022

All right, thank you for fast solution, so I closing then.

@domis77 domis77 closed this as completed Feb 3, 2022
facebook-github-bot pushed a commit that referenced this issue Feb 6, 2022
Summary:
Pull Request resolved: #1588

Add a simple test for the use-case of always transforming an atom value with an effect using `setSelf()` in `onSet()` based on concern in #1582.

Reviewed By: habond

Differential Revision: D33908670

fbshipit-source-id: 70b738bba6e3035ca90a4c489475938974df552f
facebook-github-bot pushed a commit that referenced this issue Feb 6, 2022
Summary:
Pull Request resolved: #1589

Avoid releasing retainables at the end of the batch if there were writes while processing them.  This could happen, for example, if there are effects which monitor changes and then also write state.

This error is currently superfulous for the open source release where garbage collection is enabled but not used.

Addresses #1582

Reviewed By: davidmccabe

Differential Revision: D33965759

fbshipit-source-id: 904f6e1778f7ba864cea54f0e3f2641e995834c1
@drarmstr drarmstr changed the title setSelf() in atom effect produce eleaseNodesNowOnCurrentTree error setSelf() in atom effect produce releaseNodesNowOnCurrentTree error Mar 22, 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

Successfully merging a pull request may close this issue.

2 participants