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

fix(client): exactOptionalPropertyTypes clean up #22435

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

jason-ha
Copy link
Contributor

@jason-ha jason-ha commented Sep 7, 2024

Select changes required to support new Presence package being built with exactOptionalPropertyTypes: true.

When exactOptionalPropertyTypes is false, a property specified as optional (via ?) may be assigned undefined. When true that is not allowed.

Update types to explicitly allow undefined (no effective type change as most build continues to use exactOptionalPropertyTypes: false) or to properly respect not setting undefined. The latter is an implementation change but conforms to declared specification.

Make fix(client): make IFluidDataStoreRuntime.idCompressor non-optional but retain undefined possibility. This matches existing implementations.

PrefetchDocumentStorageService - use explicit return type to avoid inference that may infer maximumCacheDurationMs as | undefined which could later be inconsistent with exactOptionalPropertyTypes: true.

Select changes required to support new Presence package being built with `exactOptionalPropertyTypes: true`

When exactOptionalPropertyTypes is false, a property specified as optional (via `?`) may be assigned `undefined`. When true that is not allowed.

Update types to explicitly allow undefined (no effective type change as build continues to use `exactOptionalPropertyTypes: false`) or to properly respect not setting `undefined`. The latter is an implementation change, but conforms to declared specification.
@jason-ha jason-ha requested a review from a team as a code owner September 7, 2024 23:35
@github-actions github-actions bot added base: main PRs targeted against main branch area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc public api change Changes to a public API labels Sep 7, 2024
…rageService

use explicit return type to avoid inference that may infer maximumCacheDurationMs as `| undefined` which is later inconsistent with `exactOptionalPropertyTypes: true`
but retain `undefined` possiblity. This matches existing implementations.
Copy link
Contributor

github-actions bot commented Sep 9, 2024

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluidframework-docs@0.25.0 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"


> fluidframework-docs@0.25.0 ci:start
> http-server ./public --port 1313 --silent


> fluidframework-docs@0.25.0 linkcheck:full
> npm run linkcheck:fast -- --external


> fluidframework-docs@0.25.0 linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

Stats:
  404342 links
    3145 destination URLs
       2 URLs ignored
       0 warnings
       0 errors


@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +345 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 460.2 KB 460.24 KB +38 Bytes
azureClient.js 558.17 KB 558.24 KB +74 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 260.96 KB 260.97 KB +17 Bytes
fluidFramework.js 401.34 KB 401.35 KB +14 Bytes
loader.js 134.24 KB 134.25 KB +14 Bytes
map.js 42.44 KB 42.44 KB +7 Bytes
matrix.js 146.59 KB 146.59 KB +7 Bytes
odspClient.js 525.47 KB 525.54 KB +74 Bytes
odspDriver.js 97.72 KB 97.76 KB +43 Bytes
odspPrefetchSnapshot.js 42.78 KB 42.81 KB +36 Bytes
sharedString.js 163.28 KB 163.29 KB +7 Bytes
sharedTree.js 391.8 KB 391.81 KB +7 Bytes
Total Size 3.3 MB 3.3 MB +345 Bytes

Baseline commit: c3a4565

Generated by 🚫 dangerJS against 646b881

@jason-ha jason-ha enabled auto-merge (squash) September 9, 2024 23:34
@jason-ha jason-ha enabled auto-merge (squash) September 10, 2024 00:08
@jason-ha jason-ha merged commit 88d652c into main Sep 10, 2024
37 checks passed
@jason-ha jason-ha deleted the test/jasonha-exactOptionalPropertyTypes-cleanup branch September 10, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants