-
Notifications
You must be signed in to change notification settings - Fork 517
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
Conversation
⯅ @fluid-example/bundle-size-tests: +486 Bytes
Baseline commit: 617ef08 |
ping me when primary reviewer has approved, merge-tree changes are fine. |
There was a problem hiding this 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", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
## 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.
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.