Tiny cleanup of PureDataObject

[CraigMacomber]

Description

Tiny cleanup of PureDataObject.

Breaking Changes

The undocumented protected property PureDataObject.initializeP is no longer exposed.
This removal does not break any documented behavior and there was no clear useful way to use this property even if relying on undocumented behavior.
finishInitialization exists and is documented to allow any synchronization that could reasonably be done using initializeP.

Reviewer Guidance

The review process is outlined on this wiki page.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

[anthony-murphy]

this is a breaking change to a legacy api, so will have to go through the breaking process which requires deprecation, and then removal in a .10 release

[CraigMacomber]

We are breaking no documented behavior. Anything using this relies entirely on undocumented behavior.

I'm not clear on the exact details of what our policy on changing undocumented behavior, but I generally thought that this was something that's allowed, especially if there is no reasonable reason anyone would ever use it, a clear better documented alternative has always existed, and its removal isn't going to cause compile errors in any reasonable case (since this isn't an interface and if used as an interface this member would be omitted anyway since its protected).

We could leave a vestigial deprecated always undefined value on the object to ensure there is no difference in the type signature level. Would that be fine? It seems pointless but would cause this change to not appear in our API reports. That doesn't seem better to me but maybe its allowed by whatever our policy is here?

It seems more likely that introducing the private initializationPromise property is a breaking change since that can cause a name collision and runtime breakage in real code not relying on undocumented behavior, but we just ignore those kinds of changes and be overly strict about the easy to nit pick stuff in API reports?

I think we need a relatively flexible definition of what is breaking driven based on documented behavior and what people might reasonably end up using, at least for any usage of non-sealed classes in the API surface: subclassing is real fragile and the API reports don't accurately capture what is actually risky and what is safe.

[CraigMacomber]

I just moved initializationPromise to use a JS private property to avoid that being a possible break (might as well, once we move to es2022 that pattern will get better minification too). No need to include an additional possible break.

[anthony-murphy]

we generally take a conservative view of all breaking changes, as before all breaks we try to ensure there is no usage before hand and that an alternative exists. Only then do we deprecate, then remove. this ensures consistency for our legacy partners in that they can safely take all non-.10 minor and there should be minimal chance of issues, and we batch all changes that could cause issues in our .10 release so partners can craft semvers, and plan around these possibly more disruptive releases. @jason-ha could probably provide more guidance.

[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:
  170508 links
    1603 destination URLs
    1842 URLs ignored
       0 warnings
       0 errors