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 Vite builder source loader plugin #19610

Conversation

joshwooding
Copy link
Member

Issue: #19554

What I did

Added getOptions to the mocked class loader.

sourceloader doesn't have any types so I added expect error.

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@joshwooding joshwooding requested a review from IanVS October 24, 2022 21:29
@joshwooding joshwooding force-pushed the fix-vite-builder-source-loader-plugin branch from 575a656 to abdbe93 Compare October 28, 2022 22:32
@joshwooding joshwooding requested a review from IanVS October 28, 2022 22:42
@joshwooding joshwooding force-pushed the fix-vite-builder-source-loader-plugin branch from abdbe93 to 685f8de Compare October 30, 2022 16:06
@joshwooding joshwooding force-pushed the fix-vite-builder-source-loader-plugin branch from 685f8de to 73b9f2d Compare October 30, 2022 16:06
@IanVS
Copy link
Member

IanVS commented Oct 31, 2022

Will this still be required if we go with #19680?

@joshwooding
Copy link
Member Author

Will this still be required if we go with #19680?

Nope, this work led to #19680 being done. I guess depending on timeframes of it being merged this can be seen as a "quick fix"

@shilman
Copy link
Member

shilman commented Nov 1, 2022

Hi Josh, I'm merging #19680 so I'm going to close this. Can you please integrate that instead? Happy to chat about it on Discord and/or pair if it helps.

@shilman shilman closed this Nov 1, 2022
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.

3 participants