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

Upgrade to vue 3 #91

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Upgrade to vue 3 #91

wants to merge 11 commits into from

Conversation

bkjohnson
Copy link

@bkjohnson bkjohnson commented Jan 22, 2024

Vue 2 reached end of life in December, so this upgrades the package to Vue 3 following the vue CLI migration instructions. Moving to vue 3 gets us onto Vite. The priorities for this work were to keep the local npm scripts running while also making sure that the cli still worked when the package is used as a dependency.

I've also bumped the tailwind version.

To test these changes I did the following:

  • ran npm run dev in the package and verified that the page looked fine in the browser with the sample config
  • ran ./cli/index.js -c tailwind.config.sample.js to simulate using the cli when the package is installed
  • used the cli while yarn linked to another local package to run the server and view the config in a browser
  • used the cli to export the viewer and served the static pages from the output directory with npx http-serve .

Dark mode still works, and I was able to see the config values that I expected to see.

This is following the instructions at [this article][1]. I removed
vue-intersect because it wasn't compatible with vue3 - I may go back and
figure something out.

[1]: https://vueschool.io/articles/vuejs-tutorials/how-to-migrate-from-vue-cli-to-vite/
There is still a Proxy client side that I think is causing problems
I couldn't figure out how to get vue to work with the async dynamic
import so I created the index file instead. There is a problem with the
Transitions component which I will try to fix next
This just leverages the vite dev server to be very similar to the `dev`
npm script. I used `yarn link` to test it with another local package and
found that I needed to use some absolute paths to ensure that the right
config got used. Next will be a similar treatment for the export
command.
I think we'll need to publish the src because vite is going to build it
for the cli commands.
return defineConfig({
plugins: [vue()],
define: {
__TAILWIND_CONFIG__: JSON.stringify(resolveConfig(conf.default)),
Copy link
Author

Choose a reason for hiding this comment

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

Using vite to pass in a stringified version of the config instead of running a separate koa server with the config.json endpoint is one of the bigger changes here. This impacts not only how we get the config in Canvas.vue, but the shape of the build as well. Anytime we run a build or run the preview server, the tailwind config will be included in the JS bundle.

import { resolve } from 'path'

export default {
darkMode: 'class',
Copy link
Author

Choose a reason for hiding this comment

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

This allows us to toggle dark mode manually with the newer version of tailwind.

Comment on lines +5 to +7
content: [
`${resolve(__dirname)}/src/**/*.vue`,
`${resolve(__dirname)}/src/**/*.js`,
Copy link
Author

Choose a reason for hiding this comment

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

content is a newer tailwind config option to replace purge but the reason I'm resolving the globs here is to make sure tailwind runs correctly when we run the vite server from the tailwind-config-viewer cli.

@@ -63,18 +56,17 @@

<script>
import defu from 'defu'
import Intersect from 'vue-intersect'
Copy link
Author

Choose a reason for hiding this comment

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

This wasn't compatible with vue 3 so I just removed it. It seems like it's for optimization reasons, but if we still want that we may need to use this function from vue core.

@@ -101,7 +93,7 @@ export default {

methods: {
sectionComponent (component) {
return require(`./Sections/${component}.vue`).default
return sections[component]
Copy link
Author

Choose a reason for hiding this comment

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

I added the sections import because I couldn't get the dynamic import to work right with the other updates.

const config = await fetch(window.__TCV_CONFIG.configPath)
this.config = await config.json()
this.config = JSON.stringify(__TAILWIND_CONFIG__)
this.config = JSON.parse(this.config)
Copy link
Author

Choose a reason for hiding this comment

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

Here's where vite gives us the stringified tailwind config as part of the build instead of us fetching it from the separate server. I tried to do the parsing and stringifying in one line but I was seeing unexpected things when I logged to the console - I'm not sure why 🤷

]
plugins: {
tailwindcss: {
config: resolve(__dirname, './tailwind.config.js')
Copy link
Author

Choose a reason for hiding this comment

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

This is also because when we run the cli, vite will use this file and we want it to use our tailwind config, not the tailwind config of the project that's using this package.

"cli",
"dist",
"src",
Copy link
Author

Choose a reason for hiding this comment

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

Because vite builds the tailwind config into the output instead of getting it at runtime from a separate server, I think we need to publish the source.

@bkjohnson bkjohnson marked this pull request as ready for review January 22, 2024 04:02
@bkjohnson bkjohnson mentioned this pull request Jan 22, 2024
@rogden
Copy link
Owner

rogden commented Jan 25, 2024

@bkjohnson Thank you! Will take a look this weekend. Appreciate your efforts.

@ineshbose
Copy link
Contributor

Thank you! Will take a look this weekend. Appreciate your efforts.

Small bump reminder in case! Hope you get time to look at this for this weekend. 🙂

@bkjohnson bkjohnson closed this Apr 29, 2024
@ineshbose
Copy link
Contributor

Disappointed to see this close. :(

@bkjohnson
Copy link
Author

It seemed like @rogden wasn't interested, and after his work on the 2.0 release there were some merge conflicts that I wasn't motivated to deal with.

@rogden
Copy link
Owner

rogden commented Apr 30, 2024

@bkjohnson Thanks for the work on this. I do think upgrading to Vue 3 is important in the mid term and appreciate the work. While I'm not certain I would ditch Koa as it is used to serve the app to the Nuxt Tailwind module, there are some other parts of this work that would make sense to incorporate. I'm happy to take over the work when I get time if you wouldn't mind leaving it open.

@bkjohnson
Copy link
Author

That's fine with me.

@bkjohnson bkjohnson reopened this May 4, 2024
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.

4 participants