Deprecate types in tree and re-export from fluid-framework

[sonalideshpandemsft]

Add a changeset to deprecate types from @fluidframework/tree. Modify fluid-framework to export the non-deprecated APIs from @fluidframework/core-interfaces.

The exports have been reorganized: APIs intended for export from fluid-framework are now in indexCommonApi.ts, while the remaining are placed in index.ts.

This change addresses the issue of duplicate exports in fluid-framework for the Listenable, Listener, Off, and IsListener types, which are now moved to @fluidframework/core-interfaces and deprecated in @fluidframework/tree.

Customers can still import these types from @fluidframework/tree, but they are marked as deprecated there. It is recommended to import them directly from @fluidframework/core-interfaces instead.

indexCommonApi.ts excludes legacy alpha and deprecated APIs.

ADO#25440

[msfluid-bot]

Code Coverage Summary

No packages impacted by the change.

---

Baseline commit: 5d813e5
Baseline build: 310291
Happy Coding!!

Code coverage comparison check passed!!

[msfluid-bot]

⯅ @fluid-example/bundle-size-tests: +245 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	467.08 KB	467.11 KB	⯅ +35 Bytes
azureClient.js	563.85 KB	563.9 KB	⯅ +49 Bytes
connectionState.js	724 Bytes	724 Bytes	■ No change
containerRuntime.js	263.27 KB	263.29 KB	⯅ +14 Bytes
fluidFramework.js	428.93 KB	428.94 KB	⯅ +14 Bytes
loader.js	134.18 KB	134.19 KB	⯅ +14 Bytes
map.js	42.71 KB	42.71 KB	⯅ +7 Bytes
matrix.js	150.15 KB	150.16 KB	⯅ +7 Bytes
odspClient.js	530.45 KB	530.5 KB	⯅ +49 Bytes
odspDriver.js	98.65 KB	98.67 KB	⯅ +21 Bytes
odspPrefetchSnapshot.js	43.04 KB	43.05 KB	⯅ +14 Bytes
sharedString.js	166.23 KB	166.24 KB	⯅ +7 Bytes
sharedTree.js	419.39 KB	419.39 KB	⯅ +7 Bytes
Total Size	3.38 MB	3.38 MB	⯅ +245 Bytes

---

Baseline commit: 5d813e5

Generated by 🚫 dangerJS against 850f4e3

[github-actions]

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output

> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170006 links
    1595 destination URLs
    1825 URLs ignored
       0 warnings
       0 errors

[CraigMacomber]

This change does not seem to be described by the PR description. If it needs to be part of this API deprecating event APIs, it would be good to document why this change is being done as part of this in the PR description.

Also it should be documented in the code (here and/or in indexCommonApi) why indexCommonApi exists so people reading the code know what its for and how to maintain it.

[CraigMacomber]

Note that without knowing why this change is being made, I can't comment on if it's a good idea or done correctly.

Generally, I don't like submodule exports like this since they can very easily break our API reporting / rollup generation tools (since they don't look inside modules like this) and can easily generate errors when packages reexport types from nested modules.

Most of these exports seem to be internal APIs, which I don't see why you would want to modify this way, but if any of them are more public than internal, I think we will leak the internal ones into the rollup with them due to tooling limitations.

[CraigMacomber]

Also #23256 removes many of these exports (they are only use by the dev tool): would be nice to do this change after that one if practical.

Edit: This change has landed, removing many of these APIs.

[CraigMacomber]

If the goal here is to avoid Multiple exports of name 'x'.eslintimport/export, be aware that that is not a JavaScript or a TypeScript error. That is an eslint error used to detect when people have conflicting exports on accident, letting them know that some of the exports that would otherwise been included in the * export were skipped. Given that this is exactly what you want to happen, I suggest simply suppressing the lint error, and adding a comment about the suppression along the lines of "This reexports all non-conflicting APIs from tree. The event APIs are known to conflict, and this is intended as the exports via core-interfaces are preferred over the deprecated ones from tree".

This should avoid the need to mess with trees's exports (which when undone, fixes the fact that you haven't documented why you had to change them)

[jason-ha]

The refactor was indeed in response to the multiple same export issue. The original thread is #23183 (comment) and none of us knew offhand what the specified or most importantly actual behavior was for multiple of the same export. Seemed better to avoid the situation altogether. This current pattern appears compatible with our export verifications, and I don't see holes in coverage. Though yes, the pattern should be clearly documented in the code.
Alternatively, we can simplify with lint disables and added comments explaining exactly what happens.

[sonalideshpandemsft]

Opened a new PR for this change: #23313

[jason-ha]

I am going to unresolve this issue because we need to pick one or the other.