Skip to content

fix(apollo-usage-report): fix issue 3952 #3956

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Sitronik
Copy link

@Sitronik Sitronik commented Apr 3, 2025

This changes fixed issue #3952

In the proposed solution, I made the following changes:

  1. Added a pendingSchemaChange variable that will store schema information if onSchemaChange is called before onYogaInit.
  2. Created a processSchemaChange function that handles schema changes only when yoga is already initialized.
  3. Modified the onSchemaChange handler so that it:
  • If yoga is not yet initialized, stores schema information in pendingSchemaChange
  • If yoga is already initialized, handles the schema change immediately
  1. Extended the onYogaInit handler so that it checks for pending schema changes and handles them after yoga is initialized.
  2. Also moved logger initialization inside onYogaInit to avoid issues with using yoga.logger before initialization.

@ardatan
Copy link
Member

ardatan commented Apr 4, 2025

Thanks for the issue and the PR! But in order to prevent the future regressions and for us to see and reproduce the issue, could you add tests?

@Sitronik
Copy link
Author

Sitronik commented Apr 4, 2025

Thanks for the issue and the PR! But in order to prevent the future regressions and for us to see and reproduce the issue, could you add tests?

Yes, of course, I'll add unit tests a bit later

@Sitronik
Copy link
Author

Sitronik commented Apr 8, 2025

@ardatan Hi, were you able to reproduce the issue with the added tests?

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