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: revert the lazy import of graphql #2298

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

Conversation

grebenyuksv
Copy link

@grebenyuksv grebenyuksv commented Sep 30, 2024

This reverts the recent attempts to import graphql lazily, which we agreed to do in #2250 (comment).

@grebenyuksv grebenyuksv changed the title Fix/graphql import fix: revert lazy imports of graphql Sep 30, 2024
@grebenyuksv grebenyuksv marked this pull request as ready for review September 30, 2024 11:44
@kettanaito
Copy link
Member

Thanks for opening this, @grebenyuksv. Can you please remind me what issue are you facing with the dynamic require?

I have a suspicion that merging this will cause errors for ESM consumers. Importing anything from the root results in an import to parseGraphQL, and then errors that the graphql package is not installed.

Copy link
Member

@kettanaito kettanaito left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you, @grebenyuksv.

I confirm this is how it used to be before the lazy-loading disaster (ref).

@kettanaito kettanaito changed the title fix: revert lazy imports of graphql fix: revert the lazy import of graphql Oct 21, 2024
@kettanaito
Copy link
Member

I'm concerned that this change will make #2248 an issue again.

Perhaps we should take a closer look at why your use case errors, @grebenyuksv.

@grebenyuksv
Copy link
Author

Thanks for your review, @kettanaito! Let's close it for now. I've parked that Storybook project where the dynamic import doesn't work and, as I said, I would rather bet that the problem is in other tooling, not in MSW.

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