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

expose building blocks #2974

Conversation

dmaskasky
Copy link
Collaborator

Summary

Expose buildingBlocks by passing in to buildStore.

Background: We sometimes want to use a custom function in place of a building block when creating a new store. This PR is one approach that enables this functionality.

Check List

  • pnpm run fix:format for formatting code and docs

Copy link

vercel bot commented Feb 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jotai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 2:44am

Copy link

codesandbox-ci bot commented Feb 4, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link

pkg-pr-new bot commented Feb 4, 2025

Open in Stackblitz

More templates

npm i https://pkg.pr.new/jotai@2974

commit: 087d9a9

@dmaskasky dmaskasky changed the title Buildstore buildingblocks expose building blocks Feb 4, 2025
@dmaskasky dmaskasky changed the base branch from main to feat/core/expose-internals-instead-of-derive February 4, 2025 20:01
Copy link

github-actions bot commented Feb 4, 2025

LiveCodes Preview in LiveCodes

Latest commit: 087d9a9
Last updated: Feb 5, 2025 2:43am (UTC)

Playground Link
React demo https://livecodes.io?x=id/DQZ3YKV65

See documentations for usage instructions.

@dai-shi
Copy link
Member

dai-shi commented Feb 4, 2025

Do you have any specific use cases in your mind?
I almost like this flexibility, except that this exposes getStore workaround ouside internals.ts. I wonder if there's a solution to it.

@dmaskasky
Copy link
Collaborator Author

Do you have any specific use cases in your mind? I almost like this flexibility, except that this exposes getStore workaround ouside internals.ts. I wonder if there's a solution to it.

Previously, we had the ability to build a store with a custom getAtomState, setAtomState. jotai-scope uses this when creating scoped stores.

With the new implementation, I think jotai-scope will likely need access to modify addtional functions such as flushPending and invalidateDependents to provide scope-specific behavior.

While its more work to build a custom store, I think its since these apis are INTERNAL anyway.

@dai-shi
Copy link
Member

dai-shi commented Feb 5, 2025

I'm still not sure if jotai-scope needs it, but I would like to let you worry about it. So exposing them is fine. I'll tweak a little bit as I didn't like the role of storeArgs already.

@dmaskasky
Copy link
Collaborator Author

dmaskasky commented Feb 5, 2025

I'm still not sure if jotai-scope needs it, but I would like to let you worry about it. So exposing them is fine. I'll tweak a little bit as I didn't like the role of storeArgs already.

I should be able to remove dependence on unstable_is soon.

@dai-shi
Copy link
Member

dai-shi commented Feb 5, 2025

I'll tweak a little bit as I didn't like the role of storeArgs already.

b24ad70

@dai-shi
Copy link
Member

dai-shi commented Feb 5, 2025

Let me take care of it.

@dai-shi
Copy link
Member

dai-shi commented Feb 5, 2025

I think this is coming full circle, but as we removed getAtomState, it's more like a level-up spiral. You may not like the flat building blocks array, but this is my strong opinion.

@dai-shi
Copy link
Member

dai-shi commented Feb 5, 2025

/ecosystem-ci run

Copy link

github-actions bot commented Feb 5, 2025

Ecosystem CI Output

---- Jotai Ecosystem CI Results ----
{
  "bunshi": "PASS",
  "jotai-devtools": "PASS",
  "jotai-effect": "PASS",
  "jotai-scope": "FAIL"
}

@dai-shi dai-shi merged commit ca35233 into pmndrs:feat/core/expose-internals-instead-of-derive Feb 5, 2025
44 checks passed
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.

2 participants