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

Tech: make the version range of vite also allow for vite 4 #20294

Closed
wants to merge 4 commits into from

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Dec 15, 2022

Issue: support vite 4?

What I did

I allowed vite in the version range, this fixes yarn start. (in our contribution.md)
It might also just be the right thing to do?

@ndelangen ndelangen self-assigned this Dec 15, 2022
@ndelangen ndelangen changed the title make the version range of vite also allow for vite 4 Tech: make the version range of vite also allow for vite 4 Dec 15, 2022
@IanVS
Copy link
Member

IanVS commented Dec 15, 2022

It looks like this is an alternate solution to #20281? Is there a reason not to go the route I took there? I'd prefer to keep vite as a peer dependency, as it was in 6.5, and I think we can do it without problem now that we are not injecting the react plugin to handle mdx.

@ndelangen
Copy link
Member Author

ndelangen commented Dec 16, 2022

No there isn't. @JReinhold asked me to look into his svelte PR, and when I did I noticed the yarn failure, so I fixed it, and extracted the fix out into it's own PR here.

@ndelangen ndelangen closed this Dec 16, 2022
@ndelangen ndelangen reopened this Dec 16, 2022
@ndelangen
Copy link
Member Author

@IanVS I do not know what's going on with that PR, I'm happy to prefer that one over this one, for sure.
But I'd love to have yarn start and vite sandboxes in general work without linking, which is all I wanted to achieve in this PR.

@ndelangen
Copy link
Member Author

I can't reproduce this unit-test fail locally. Argh I feel like giving up on this.

@IanVS
Copy link
Member

IanVS commented Dec 16, 2022

I found the issue in the other PR(#20281). react-docgen wasn't being pinned correctly.

@ndelangen
Copy link
Member Author

ok, I'll close it again

@ndelangen ndelangen closed this Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants