breaking(internals): avoid getInternalBuildingBlock function#3293
breaking(internals): avoid getInternalBuildingBlock function#3293
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. |
commit: |
|
Size Change: -92 B (-0.09%) Total Size: 104 kB
ℹ️ View Unchanged
|
|
| Playground | Link |
|---|---|
| React demo | https://livecodes.io?x=id/3Z5PJFE6L |
See documentations for usage instructions.
|
My initial thought is that rest/spread adds overhead to these internal functions. |
|
This gets closer to #3280. One of meaningful changes is that I dislike putting store in buildingBlocks. |
| // store api | ||
| store, | ||
| ] satisfies BuildingBlocks | ||
| ).map((fn, i) => buildArgs[i] || fn) as BuildingBlocks |
There was a problem hiding this comment.
Passing store in buildArgs is technically possible, but should not affect anything.
This is a common pattern. The intent is not to overwrite the store (since that is what we are building).
const buildingBlocks = getBuildingBlocks(store)
buildingBlocks[0] = new AtomState()
const store2 = buildStore(...buildingBlocks)I recommend putting buildingBlocks[29] = store after this line to overwrite buildArgs[29].
There was a problem hiding this comment.
I don't like it then. If we specialize store like that, I wouldn't put it into the building blocks.
There was a problem hiding this comment.
Then let's void it from the buildArgs.
There was a problem hiding this comment.
I mean I don't like adding store in buildingBlocks.
| } | ||
| try { | ||
| return atomWrite(store, atom, getter, setter, ...args) | ||
| return atomWrite(buildingBlocks, atom, getter, setter, ...args) |
There was a problem hiding this comment.
Or should we also make atomWrite non-variadic?
There was a problem hiding this comment.
I don't see any use cases, but feels consistent for all atom interceptors.
|
@arjunvegda @dmaskasky |
| ) => Readonly<BuildingBlocks> | ||
| type AbortHandlersMap = WeakMapLike<PromiseLike<unknown>, Set<() => void>> | ||
| type RegisterAbortHandler = <T>( | ||
| store: Store, |
There was a problem hiding this comment.
we should still pass the store as the second param.
|
I'll work on them. |
Ok, and we should remember to set the minimum version of Jotai peer dep to this version. |
|
@dmaskasky reported this breaking change makes jotai-scope unmaintainable. Let's start over when we get time. @arjunvegda |
I was able to reach a solution for jotai-scope and jotai-effect. |
|
wow, that's great to hear. |
|
/ecosystem-ci run |
Ecosystem CI Output |
2 participants
alternative to #3280. Though, it became pretty similar.