Make isFluidHandle public #22029
Merged
Make isFluidHandle public #22029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 881fefc The changes in this PR will be included in the next version bump. This PR includes changesets to release 157 packages
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 |
github-actions
bot
added
area: framework
⯅ @fluid-example/bundle-size-tests: +245 Bytes
Baseline commit: fd06b93 |
tylerbutler
reviewed
Jul 26, 2024
| @@ -56,6 +56,8 @@ export type { | |||
| FluidObjectProviderKeys, // Used by FluidObject | |||
| } from "@fluidframework/core-interfaces"; | |||
|
|
|||
| export type { isFluidHandle } from "@fluidframework/runtime-utils"; | |||
There was a problem hiding this comment.
This is a new export source for fluid-framework. @Josmithr do we need to make other config changes to ensure the docs are built correctly?
There was a problem hiding this comment.
Nope :) API-Extractor in this package is configured to bundle all @fluidframework packages automatically (assuming they are correctly declared as a direct dependency, which this now is).
There was a problem hiding this comment.
In this case you can tell because the actual declaration ends up in the API reports in this package, rather than appearing as a re-export from an external package.
Josmithr
reviewed
Jul 26, 2024
Josmithr
reviewed
Jul 26, 2024
| @@ -1818,130 +1818,6 @@ importers: | |||
| specifier: ^5.8.0 | |||
| version: 5.10.0 | |||
|
|
|||
| examples/benchmarks/bubblebench/shared-tree-flex-tree: | |||
There was a problem hiding this comment.
Nit: These changes seem unrelated.
There was a problem hiding this comment.
It looks like they are from a PR I did the other day, where I removed the shared-tree-flex-tree package. So this is probably good to check in. Is there something I need to do besides run pnpm i in order to regenerate the pnpm-lock? I thought that running pnpm i would have done this for me, but I'm pretty sure I ran pnpm i and it did not.
There was a problem hiding this comment.
If you want the lock file to update, you need to run pnpm i --no-frozen-lockfile now. That said if doing so would make a change, I'd expect pnpm i to error and tell you that,. I guess maybe there is some subset of changes which would update, but doesn't error?
Josmithr
approved these changes
Jul 26, 2024
Co-authored-by: Joshua Smithrud <[email protected]>
This was referenced Aug 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This makes isFluidHandle public.
An alternative API, using Symbol.hasInstrance and
instancof IFluidHandleis not possible here, since there already is non-type value exported under the name IFluidHandle (its the string "IFluidHandle") which can not have the symbol added to it.This leaves the existing isFluidHandle function seeming like the only real API option.
Detecting handles correctly requires knowing implementation details of handle and dealing with the possibility of old in memory handle representations, so we don't want customers rolling their own. As there are use-cases for handle detection, we should probably expose this API to do it robustly.
Reviewer Guidance
The review process is outlined on this wiki page.
Do we want to make this public? I think so, but that should be covered by the review. We could also go to alpha or beta first. THis seems pretty safe to stabilize though.