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

Fixing some warnings due to union types #8106

Merged
merged 4 commits into from
Jan 9, 2025
Merged

Fixing some warnings due to union types #8106

merged 4 commits into from
Jan 9, 2025

Conversation

joehan
Copy link
Contributor

@joehan joehan commented Jan 8, 2025

Description

Fixing up some issues from the recent 'ajv' upgrade - namely:

  • AllowUnionTypes:true - typescript-json-schema can generate these, so we need to allow them.
  • Actually depend on ajv-formats - this slipped past our no-undeclared-imports check because it was a require before.

Scenarios Tested

Before:
Screenshot 2025-01-08 at 10 01 13 AM
After:
Screenshot 2025-01-08 at 10 01 20 AM

@joehan joehan requested a review from bkendall January 8, 2025 18:07
@bkendall
Copy link
Contributor

bkendall commented Jan 9, 2025

Looks like you've got some issues to sort out before this is ready... :/

@joehan
Copy link
Contributor Author

joehan commented Jan 9, 2025

Looks like you've got some issues to sort out before this is ready... :/

I believe most of these failures were actually caused by an NPM outage today - rerunning the tests now to confirm. Will fix and rerequest later if there are still issues

@joehan
Copy link
Contributor Author

joehan commented Jan 9, 2025

The test failures are unrelated and on non-blocking tests. The VSCode failures should be fixed by #8107 and #8108 , and the data connect e2e test is flaky.

@joehan joehan removed the request for review from bkendall January 9, 2025 18:17
@joehan joehan requested a review from bkendall January 9, 2025 19:26
Copy link
Contributor

@bkendall bkendall left a comment

Choose a reason for hiding this comment

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

Seems fine to me - though there's a lot removed from firebase-config, so maybe just confirm that that's cool

@joehan
Copy link
Contributor Author

joehan commented Jan 9, 2025

Seems fine to me - though there's a lot removed from firebase-config, so maybe just confirm that that's cool

Sanity checked it and it looks like it is still working correctly. The reduced file length is because the new version of typescript-json-schema makes (much) better use of $ref to avoid redundancies.

@joehan joehan merged commit be02559 into master Jan 9, 2025
44 of 46 checks passed
@joehan joehan deleted the jh-stderr-spam branch January 10, 2025 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants