Skip to content

Throw error when not contained#200

Merged
albertogasparin merged 1 commit intomasterfrom
tweak/contained-error
May 18, 2023
Merged

Throw error when not contained#200
albertogasparin merged 1 commit intomasterfrom
tweak/contained-error

Conversation

@albertogasparin
Copy link
Collaborator

@albertogasparin albertogasparin commented May 17, 2023

The new Store API, containedBy, allows us to signal whenever a store is being used in global context but the creator intended it to be used behind a container.
This is especially helpful to detect leaking stores, where the UI might still work but the data accidentally will be retained in the global space.
Closes #190

initStore = (key, Store, fromContainer) => {
const { initialState, actions } = Store;

if (Store.containedBy && !fromContainer) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's not checking that container is the one indicated by containedBy, allowing any other container to be present.
Question - how I can disable or satisfy this condition in tests, where isolation might be not as required, or where I don't want to fix too many things at once?

Copy link
Collaborator Author

@albertogasparin albertogasparin May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowing any other container to be present

Only the ones targeting the same type! The store creation is triggered by a matching container, not any container.

how I can disable or satisfy this condition in tests

There are 2 patterns devs can use in tests:

  • use the containedBy container directly
  • use a new createContainer(Store), which also allows to override onInit/onUpdate

The latter might be better for storybooks/tests, but as long as there is a container that captures the store state somewhere in the tree, it will work without complaining.
This should also help pushing teams writing tests without accidentally leaking to global when containedBy is used.

@albertogasparin albertogasparin merged commit ea83ff7 into master May 18, 2023
@albertogasparin albertogasparin deleted the tweak/contained-error branch May 18, 2023 00:01
@anacierdem anacierdem mentioned this pull request May 22, 2023
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 this pull request may close these issues.

Non global states

2 participants