Add all referenced fluid API to fluid-framework rollup package #20554

CraigMacomber · 2024-04-09T18:57:27Z

Description

This adds exports to the fluid-framework package until all the imports from fluid framework are gone from its API report.

This would avoid errors where using some APIs (ex: exporting them from packages in apps) may warn about types not being namable, or being unable to explicitly type some values without additional imports.

Additionally this puts the transitively reachable portion of our public API in one place where it can be reviewed.

Reviewer Guidance

The review process is outlined on this wiki page.

It is unclear to me what the policy is for which things should be exported from fluid-framework, so I'm not sure if this change is a fix or violating some decision that was made. Clarification on this would be valuable.

…to full-framework

CraigMacomber · 2024-04-12T20:48:54Z

packages/framework/fluid-framework/api-report/fluid-framework.api.md

 import { FieldSchemaUnsafe as FieldSchemaUnsafe_2 } from './typesUnsafe.js';
-import { FluidObject } from '@fluidframework/core-interfaces';
-import { IChannel } from '@fluidframework/datastore-definitions';
-import type { IErrorBase } from '@fluidframework/core-interfaces';
-import { IEvent } from '@fluidframework/core-interfaces';
-import { IEventProvider } from '@fluidframework/core-interfaces';
-import { IFluidHandle } from '@fluidframework/core-interfaces';
-import { IFluidLoadable } from '@fluidframework/core-interfaces';
-import { ISharedObjectKind } from '@fluidframework/shared-object-base';
+import { FluidObject as FluidObject_2 } from '@fluidframework/core-interfaces';
+import { IFluidHandle as IFluidHandle_2 } from '@fluidframework/core-interfaces';
+import { IFluidLoadable as IFluidLoadable_2 } from '@fluidframework/core-interfaces';


This is strange/broken. API extractor bug? Rollup issue?

See microsoft/rushstack#4654

This does seem problematic (wonder how it would affect generated doc pages). Any workaround? If we enforce consistently importing from /internal everywhere in our repo for example?

…to full-framework

packages/framework/fluid-framework/api-report/fluid-framework.api.md

packages/framework/fluid-framework/README.md

…to full-framework

CraigMacomber · 2024-04-24T18:30:12Z

https://learn.microsoft.com/en-us/azure/azure-fluid-relay/concepts/version-compatibility#compatibility-table is relevant as we document our compat in terms of the fluid-framework package, so rolling these all up into one place robustly avoids the issue of what the implacied support/compat of the packages referenced by fluid-framework is. It also reduces the risk of mismatched point releases since fluid-framework gains a direct dependency on the packages it implicitly depends on.

…to full-framework

ChumpChief

I think the main thing is that we need to clarify our support promise for these - I had been assuming we'd add them and fully support them (as they are accessible from the public API surface), so if we're saying it's something more nuanced as the @alpha tagging implies I think we need to get that written down.

ChumpChief · 2024-05-02T21:08:34Z

packages/framework/fluid-framework/api-report/fluid-framework.api.md

@@ -64,6 +24,194 @@ export enum AttachState {
 Detached = "Detached"
 }

+// @alpha


I think everything in this file needs to be @public to ensure we have a clear support promise (these are part of the API surface we intend you to use), otherwise it's kind of muddy. @DLehenbauer WDYT?

Or else we probably need something concrete about what "exported but @alpha" means -- available but no breaking change promise? available with full support promise?

I don't think our policy with respect to public vs alpha stuff is specific to this package, so I don't think this is the place to do that. I have separate work to document what we mean by supported and breaking changes, which can cover things like relying on undocumented behavior or non-public API not being supported and thus not being covered by our no breaking changes policy and what ever more formal policy we have for support.

I also don't see this PR as making any changes to what we support. For example before this PR IFluidHandle was referenced, and changing it could break users of this package. That is still true, users just don't have to depend on another package if they want to save a handle in a typed variable.

@ChumpChief - The @alpha tag is how we get Fluid Framework 1.0 types to show up under the 'fluid-framework/legacy' path. (It's the same as how we use @alpha elsewhere.)

The message to customers is that '/legacy' types are supported, but not recommended for new designs. We'll mark the types as @deprecated when we have a way for customers to migrate their data forward to SharedTree.

ChumpChief · 2024-05-02T21:10:20Z

packages/framework/fluid-framework/api-report/fluid-framework.api.md

+export interface AttributionPolicy {
+ attach: (client: Client) => void;
+ detach: () => void;
+ // (undocumented)


Probably all these should be documented too - obv a lot so not in this PR necessarily, but we should open some work items and consider them for 2.0.

I see this PR as a way to make these issues more discoverable, so I think we should specifically not block it on trying to fix these issues, and instead merge this PR before farming out work to fix issues like this.

ChumpChief · 2024-05-02T21:16:19Z

packages/framework/fluid-framework/api-report/fluid-framework.api.md

 import { FieldSchemaUnsafe as FieldSchemaUnsafe_2 } from './typesUnsafe.js';
-import { FluidObject } from '@fluidframework/core-interfaces';
-import { IChannel } from '@fluidframework/datastore-definitions';
-import type { IErrorBase } from '@fluidframework/core-interfaces';
-import { IEvent } from '@fluidframework/core-interfaces';
-import { IEventProvider } from '@fluidframework/core-interfaces';
-import { IFluidHandle } from '@fluidframework/core-interfaces';
-import { IFluidLoadable } from '@fluidframework/core-interfaces';
-import { ISharedObjectKind } from '@fluidframework/shared-object-base';
+import { FluidObject as FluidObject_2 } from '@fluidframework/core-interfaces';
+import { IFluidHandle as IFluidHandle_2 } from '@fluidframework/core-interfaces';
+import { IFluidLoadable as IFluidLoadable_2 } from '@fluidframework/core-interfaces';


This does seem problematic (wonder how it would affect generated doc pages). Any workaround? If we enforce consistently importing from /internal everywhere in our repo for example?

ChumpChief · 2024-05-02T21:19:20Z

packages/framework/fluid-framework/src/index.ts

+export type {
+ EventEmitterEventType,
+ TypedEventEmitter,
+ // It is unclear why ae-forgotten-export requires this since its not referenced.


TypedEventEmitter references it

If you search the API report for it there are no usages so it should not need to be imported. This is entangled with the strangeness that the above two types are not getting inclined in the report like everything else (if that was fixed, then this would be referenced). I'll update the comments to make that more clear.

comments have been updated.

ChumpChief · 2024-05-02T21:22:47Z

packages/framework/fluid-framework/api-report/fluid-framework.api.md

@@ -250,6 +692,226 @@ export interface IDisposable {
 [disposeSymbol](): void;
 }

+// @public
+export interface IDisposable_2 {


This seems unfortunate that IDisposable and IDisposable_2 are different? Is there something we can do to disambiguate these more nicely, merge them, etc.?

We have existing internal work items about that, see https://dev.azure.com/fluidframework/internal/_workitems/edit/6591/ which I have also just updated.

…to full-framework

msfluid-bot · 2024-05-03T20:50:42Z

⯅ @fluid-example/bundle-size-tests: +2.99 KB

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	454 KB	454.27 KB	⯅ +275 Bytes
azureClient.js	550.34 KB	550.62 KB	⯅ +290 Bytes
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	257.08 KB	257.33 KB	⯅ +258 Bytes
fluidFramework.js	344.1 KB	344.67 KB	⯅ +580 Bytes
loader.js	132.83 KB	132.83 KB	■ No change
map.js	41.15 KB	41.4 KB	⯅ +256 Bytes
matrix.js	143.32 KB	143.58 KB	⯅ +264 Bytes
odspClient.js	518.83 KB	519.12 KB	⯅ +290 Bytes
odspDriver.js	97.23 KB	97.23 KB	■ No change
odspPrefetchSnapshot.js	42.1 KB	42.1 KB	■ No change
sharedString.js	161.05 KB	161.32 KB	⯅ +270 Bytes
sharedTree.js	344.1 KB	344.67 KB	⯅ +580 Bytes
Total Size	3.16 MB	3.17 MB	⯅ +2.99 KB

Baseline commit: 350c06a

Generated by 🚫 dangerJS against 9445be5

Add all referenced fluid API to fluid-framework rollup package

029055a

github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct dependencies Pull requests that update a dependency file public api change Changes to a public API base: main PRs targeted against main branch labels Apr 9, 2024

CraigMacomber added 2 commits April 12, 2024 13:34

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

2d98fa3

…to full-framework

Update

082531f

CraigMacomber commented Apr 12, 2024

View reviewed changes

CraigMacomber marked this pull request as ready for review April 12, 2024 21:25

CraigMacomber added 2 commits April 22, 2024 23:35

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

6e0d520

…to full-framework

add missing exports

ebc1d52

CraigMacomber commented Apr 23, 2024

View reviewed changes

packages/framework/fluid-framework/api-report/fluid-framework.api.md Outdated Show resolved Hide resolved

CraigMacomber added 2 commits April 23, 2024 09:59

Add missing internal reexports

d912d35

clarify readme

ccb6ded

jatgarg reviewed Apr 23, 2024

View reviewed changes

packages/framework/fluid-framework/api-report/fluid-framework.api.md Outdated Show resolved Hide resolved

CraigMacomber commented Apr 23, 2024

View reviewed changes

packages/framework/fluid-framework/README.md Outdated Show resolved Hide resolved

CraigMacomber added 2 commits April 23, 2024 12:50

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

2bcd16a

…to full-framework

Enforce reexports with api-extractor

32451fa

CraigMacomber mentioned this pull request Apr 29, 2024

Split public and internal IFluidHandle APIs #20123

Merged

CraigMacomber added 6 commits May 2, 2024 12:03

Remove readme change: its split into another PR

b103596

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

8b647a8

…to full-framework

update api report

77277a2

fix build

5c3be61

Add note about validating legacy exports

260df05

Merge branch 'main' into full-framework

2b40b3f

ChumpChief reviewed May 2, 2024

View reviewed changes

CraigMacomber added 3 commits May 2, 2024 14:49

More docs

bbcb6b2

Merge branch 'main' into full-framework

1f99d88

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

9445be5

…to full-framework

CraigMacomber mentioned this pull request May 15, 2024

Type Erase Shared object kinds #21081

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add all referenced fluid API to fluid-framework rollup package #20554

Add all referenced fluid API to fluid-framework rollup package #20554

CraigMacomber commented Apr 9, 2024

CraigMacomber Apr 12, 2024

CraigMacomber Apr 23, 2024

ChumpChief May 2, 2024

CraigMacomber commented Apr 24, 2024

ChumpChief left a comment

ChumpChief May 2, 2024

CraigMacomber May 2, 2024

CraigMacomber May 2, 2024 •

edited

DLehenbauer May 4, 2024

ChumpChief May 2, 2024

CraigMacomber May 2, 2024

ChumpChief May 2, 2024

ChumpChief May 2, 2024

CraigMacomber May 2, 2024

CraigMacomber May 2, 2024

ChumpChief May 2, 2024

CraigMacomber May 2, 2024

msfluid-bot commented May 3, 2024

Add all referenced fluid API to fluid-framework rollup package #20554

Are you sure you want to change the base?

Add all referenced fluid API to fluid-framework rollup package #20554

Conversation

CraigMacomber commented Apr 9, 2024

Description

Reviewer Guidance

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CraigMacomber commented Apr 24, 2024

ChumpChief left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CraigMacomber May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msfluid-bot commented May 3, 2024

CraigMacomber May 2, 2024 •

edited