Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tree): Include list of properties that changed in nodeChanged event #22229

Merged
merged 45 commits into from
Sep 5, 2024

Conversation

CraigMacomber
Copy link
Contributor

Description

Updated version of #22043 .

See changeset for details.

Reviewer Guidance

The review process is outlined on this wiki page.

@github-actions github-actions bot added base: main PRs targeted against main branch area: dds Issues related to distributed data structures area: dds: tree area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct changeset-present public api change Changes to a public API labels Aug 16, 2024
@CraigMacomber
Copy link
Contributor Author

A usability issue with this APi has been reported:

Lack of strong typing of the keys least to string names for keys in code easily getting out of date (or having typos) with no compile errors.

There are two ways this could be improved:

  • strong types on the keys in the set (ex: derive them from the object node or nodes provided)
  • a way to refer to property key strings that is strongly typed to the object they are on and refactors properly. For example Foo.fields.bar.key possible leveraging a "FieldSchemaContextual" subclass which has the key as s strongly typed property.

CraigMacomber added a commit that referenced this pull request Sep 3, 2024
## Description

Refactor TreeNodeKernel.

Makes its events in terms of the internal anchor node events instead of
the public API surface, allowing for the public API surface to be
tweaked in node kind specific ways.

Track the disposed state more clearly.

Move generation number logic from array nodes to node kernel where other
kinds could use it (ex: to error when iterating map keys if the map was
modified) and implementation is simpler.

Changes split out from
#22229 for separate
review.
CraigMacomber added a commit that referenced this pull request Sep 3, 2024
## Description

 Improve TreeChangeEvents docs.

Split out from #22229
CraigMacomber added a commit that referenced this pull request Sep 3, 2024
## Description

Clarify docs and comments to refer to property keys not view keys.

Changes split out from
#22229 for separate
review.
CraigMacomber added a commit that referenced this pull request Sep 3, 2024
## Description

Split off from #22229
for a smaller review.
@CraigMacomber
Copy link
Contributor Author

A usability issue with this APi has been reported:

Lack of strong typing of the keys least to string names for keys in code easily getting out of date (or having typos) with no compile errors.

There are two ways this could be improved:

  • strong types on the keys in the set (ex: derive them from the object node or nodes provided)
  • a way to refer to property key strings that is strongly typed to the object they are on and refactors properly. For example Foo.fields.bar.key possible leveraging a "FieldSchemaContextual" subclass which has the key as s strongly typed property.

Fixed. Also moved API to beta incase there are more such issues.

@CraigMacomber CraigMacomber enabled auto-merge (squash) September 5, 2024 00:19
Copy link
Contributor

github-actions bot commented Sep 5, 2024

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start 1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://127.0.0.1:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"


> [email protected] ci:start
> http-server ./public --port 1313 --silent


> [email protected] linkcheck:full
> npm run linkcheck:fast -- --external


> [email protected] linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

Stats:
  378023 links
    2980 destination URLs
       2 URLs ignored
       0 warnings
       0 errors


@CraigMacomber CraigMacomber merged commit aae34dd into microsoft:main Sep 5, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: tree area: dds Issues related to distributed data structures area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch changeset-present public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants