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

Update build-tools and type tests #21087

Merged
merged 3 commits into from
May 15, 2024
Merged

Conversation

CraigMacomber
Copy link
Contributor

Description

#21070 made some improvements to type tests which were needed to regenerate the type tests for client. This change integrates that, as well as regenerates the type tests with the new generator.

Reviewer Guidance

The review process is outlined on this wiki page.

@CraigMacomber CraigMacomber requested review from a team as code owners May 15, 2024 01:19
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: propertydds area: dds: sharedstring area: dds: tree area: dev experience Improving the experience of devs building on top of fluid area: driver Driver related issues area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: odsp-driver area: runtime Runtime related issues area: server Server related issues (routerlicious) area: tests Tests to add, test infrastructure improvements, etc dependencies Pull requests that update a dependency file base: main PRs targeted against main branch labels May 15, 2024
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +486 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 453.18 KB 453.18 KB No change
azureClient.js 550.29 KB 550.5 KB +220 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 257.02 KB 257.02 KB No change
fluidFramework.js 357.54 KB 357.56 KB +22 Bytes
loader.js 132.78 KB 132.78 KB No change
map.js 41.45 KB 41.45 KB No change
matrix.js 143.67 KB 143.67 KB No change
odspClient.js 518.83 KB 518.83 KB No change
odspDriver.js 97.29 KB 97.29 KB No change
odspPrefetchSnapshot.js 42.15 KB 42.15 KB No change
sharedString.js 160.19 KB 160.19 KB No change
sharedTree.js 357.53 KB 357.55 KB +22 Bytes
Total Size 3.19 MB 3.19 MB +486 Bytes

Baseline commit: 617ef08

Generated by 🚫 dangerJS against 7d304e5

@Abe27342
Copy link
Contributor

ping me when primary reviewer has approved, merge-tree changes are fine.

Copy link
Contributor

@alexvy86 alexvy86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per offline discussion, all the already broken types seem to be expected.

For future reference, I think we could handle server separately for type-test update purposes given its separate versioning story. The changes here seem ok to me though.

@@ -86,9 +86,9 @@
"@biomejs/biome": "^1.6.2",
"@fluid-internal/mocha-test-setup": "workspace:~",
"@fluid-private/test-version-utils": "workspace:~",
"@fluid-tools/build-cli": "^0.38.0",
"@fluid-tools/build-cli": "^0.39.0-264124",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the PR, but this example seems to be the only one with a dependency on build-cli and I don't see it using it (no calls to flub for example). I kind of expected it to be doing type tests because it's still consumed by our partners, but they're disabled. @ChumpChief @tylerbutler do you know if this should be doing type tests or we disabled them on purpose?

There's a few other examples in the same situation. I'll see about removing the dep in all of them outside this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build cli is used to generate the type tests, so lots of things have it, but maybe this one is unneeded.

Generally example apps have no reason to have type tests, but I'm not sure whats going on with this package though, as its in @fluid-example, but is not private, has exports, and is not imported in any other examples.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table-document is used by internal partners for historical reasons. It is extremely unique and breaks a lot of "rules" regarding npm scope and supportability. @ChumpChief and @anthony-murphy might know more. There might also be some comments in the fluid-build config file.

@CraigMacomber CraigMacomber merged commit 32274f6 into microsoft:main May 15, 2024
45 checks passed
@CraigMacomber CraigMacomber deleted the typeTests branch May 15, 2024 17:27
kekachmar pushed a commit to kekachmar/FluidFramework that referenced this pull request May 21, 2024
## Description

microsoft#21070 made some
improvements to type tests which were needed to regenerate the type
tests for client. This change integrates that, as well as regenerates
the type tests with the new generator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: propertydds area: dds: sharedstring area: dds: tree area: dds Issues related to distributed data structures area: dev experience Improving the experience of devs building on top of fluid area: driver Driver related issues area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: odsp-driver area: runtime Runtime related issues area: server Server related issues (routerlicious) area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants