-
Notifications
You must be signed in to change notification settings - Fork 176
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
feat: vite 3 #358
feat: vite 3 #358
Conversation
- updated to the latest vite version - removed terser option and cleaned up vite config - updated vite-plugin-solid and vite-plugin-pwa version - updated pnpm-lock.yaml and yarn.lock files - added a break between videos in the newest july 2022 blog article - make routeReadyState more dry - cleaned up editorconfig - added .mdx to extensions, removed redundant options already declared globally - prettier updated Resources formatting during the build
Woah, this is an awesome PR! I'm excited to get us going on V3. This looks stable and works well for me. @modderme123 and @aquaductape if you want to have a look I'm ready to merge this in when you are. |
I think we should drop yarn.lock but we can do that after merging |
@@ -43,7 +43,6 @@ | |||
"solid-dismiss": "^1.2.1", | |||
"solid-heroicons": "^2.0.3", | |||
"solid-js": "1.4.7", | |||
"solid-jsx": "^0.9.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this was used by the mdx compiler...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not - solid-mdx is. If you look at the vite config, there's no mention of it there - so since it's not used, I removed it.
@@ -89,6 +89,7 @@ Also have a read through some of these articles: | |||
And some fantastic video clips we enjoyed: | |||
|
|||
<YouTube youTubeId="A_dUsSzxwkI" /> | |||
<br /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it is better to just have an extra line here and let md insert the linebreak?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that will work in mdx - adding spaces or a new line after a component, at least it doesn't for me, not here and neither on @mdx-js playground.
<br />
is already used in the first article from December 2021 for this purpose.
Also, @davedbase - if someone has editorConfig extension enabled, the white space is trimmed because of .editorconfig configuration. Should this be the default for md/mdx?
I can do that, I would also suggest that we add https://nodejs.org/api/packages.html#packagemanager property to package.json and set it to [email protected]. It will benefit https://nodejs.org/api/corepack.html users for now. The only bad thing here is that it's not possible to declare it like pnpm@7 - the full version must be added, and this will require frequent updating... But it only affects corepack users, so I think would be OK to have it. |
Let me know when we're ready to merge. I tested locally and it seems stable :-) |
I'm just going to do it, this looks fine to me aside. Feel free to make adjustments on main directly for the items you mentioned) |
options already declared globally