upgrade: lifecycle package upgrade for Solid 2.0#889
Conversation
🦋 Changeset detectedLatest commit: dfc3ba8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| */ | ||
| export const isHydrated = (): boolean => | ||
| !isServer && (!sharedConfig.context || (!!getListener() && createIsMounted()())); | ||
| !isServer && (!sharedConfig.hydrating || (!!getObserver() && createIsMounted()())); |
There was a problem hiding this comment.
Wouldn't that create mutliple isMounted signals in the worst case, since we're async now? Maybe flush() the signal so it is synchronous.
There was a problem hiding this comment.
flush doesn't work in onSettled or createTrackedEffect. I added a test to verify if this is a concern at all. It demonstrates the property: a second onSettled registered after createIsMounted can already read isMounted() as true, proving that setIsMounted(true) is applied synchronously within the flush which is exactly why flush() inside onSettled is maybe not needed.
This one is tricky, you may want to study the tests I added more closely and run it directly.
| import { createIsMounted, isHydrated } from "../src/index.js"; | ||
|
|
||
| describe("createIsMounted", () => { | ||
| test("createIsMounted", () => { |
There was a problem hiding this comment.
We should test that the isMounted signal is not needlessly created multiple times.
2 participants
Upgrades
@solid-primitives/lifecycleto Solid 2.0 beta.10. All API signatures (createIsMounted,isHydrated,onElementConnect) are unchanged.Breaking changes for downstream consumers:
solid-js@^2.0.0-beta.10and@solidjs/web@^2.0.0-beta.10onMount→onSettled,getListener→getObserver,sharedConfig.context→sharedConfig.hydrating,isServerfrom `@solidjs/webSummary by CodeRabbit
@solidjs/webv2.0 (beta.14) as peer dependencies.