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

feat: add Quasar Framework #208

Merged
merged 5 commits into from
Mar 19, 2024
Merged

Conversation

yusufkandemir
Copy link
Contributor

Hi, Quasar core team member here 👋

Quasar Framework uses Vite in two different Quasar flavors:

Quasar CLI with Vite uses the Vite plugin for Quasar under the hood, so covering the Vite plugin should be a good start. We have quasarframework/quasar#15755 which adds the testing setup, package scripts, etc. that will be run by the Ecosystem CI. We probably do not need to make further changes here if we add tests for Quasar CLI with Vite later, modifying the dedicated package scripts should be enough.

Please let me know if there is anything else we should do, thanks 🙂

Creating it as a draft since the upstream PR is not merged yet, but I wanted to get this out as soon as possible to gather feedback.

A note: pnpm run test quasar --release 4.3.3 works successfully, but pnpm run test quasar fails with the following error:

/Users/yusuf/dev/github/yusufkandemir/vite-ecosystem-ci/workspace/quasar/quasar $> git clean -fdxq
/Users/yusuf/dev/github/yusufkandemir/vite-ecosystem-ci/workspace/quasar/quasar $> yarn install
yarn install v1.22.19
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning " > @vitejs/[email protected]" has unmet peer dependency "vue@^3.2.25".
warning " > @vitejs/[email protected]" has unmet peer dependency "terser@^5.4.0".
warning " > @quasar/[email protected]" has unmet peer dependency "quasar@^2.8.0".
warning " > @quasar/[email protected]" has unmet peer dependency "vue@^3.0.0".
error An unexpected error occurred: "ENOENT: no such file or directory, stat '/Users/yusuf/dev/github/yusufkandemir/vite-ecosystem-ci/workspace/quasar/quasar/node_modules/vite/node_modules/rollup'".
info If you think this is a bug, please open a bug report with the information provided in "/Users/yusuf/dev/github/yusufkandemir/vite-ecosystem-ci/workspace/quasar/quasar/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
file:///Users/yusuf/dev/github/yusufkandemir/vite-ecosystem-ci/node_modules/.pnpm/[email protected]/node_modules/execa/lib/error.js:59
                error = new Error(message);
                        ^

Error: Command failed with exit code 1: yarn install
warning " > @vitejs/[email protected]" has unmet peer dependency "vue@^3.2.25".
warning " > @vitejs/[email protected]" has unmet peer dependency "terser@^5.4.0".
warning " > @quasar/[email protected]" has unmet peer dependency "quasar@^2.8.0".
warning " > @quasar/[email protected]" has unmet peer dependency "vue@^3.0.0".
error An unexpected error occurred: "ENOENT: no such file or directory, stat '/Users/yusuf/dev/github/yusufkandemir/vite-ecosystem-ci/workspace/quasar/quasar/node_modules/vite/node_modules/rollup'".

Tested with Node v18.16.0, Yarn v1.22.19, MacOS Ventura 13.3.1

repo: 'yusufkandemir/quasar',
branch: 'vite-plugin-add-tests',

@yusufkandemir yusufkandemir marked this pull request as ready for review May 8, 2023 16:00
@dominikg
Copy link
Collaborator

dominikg commented May 8, 2023

@yusufkandemir
Copy link
Contributor Author

@dominikg done 👍

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me. Please keep an eye on the #ecosystem-ci channel in Vite Land. We can start tagging you there if there is a red in Quasar before a release.

Just curious, why the build step is different when testing in vite-ecosystem-ci and in the vue-ecosystem-ci here? https://github.com/quasarframework/quasar/blob/933a997b63d148987a61788cd4aa8c25f0eb41f2/package.json#L12

yusufkandemir added a commit to quasarframework/quasar that referenced this pull request May 9, 2023
@yusufkandemir
Copy link
Contributor Author

@patak-dev

I think I'd need the roles for the mentions. So, here is my Discord handle: AzzimothTR#4420

For vite-ecosystem-ci, we specifically test the Vite plugin for the "framework" parts, such as component auto-loading, SCSS variable injection, etc. For vue-ecosystem-ci, we focus on the Vue UI library part. So, the idea is to build/test only the related parts, to consume less CI resources, and to not get sources of error mixed.

But, thanks for being curious about this because I noticed that the test script doesn't work anymore. yarn workspaces automatically link everything locally unlike pnpm which does that only when using the workspace: protocol AFAIK. At the time I created this script our workspace setup didn't have the UI package involved. So, now we need to build all the packages for both ecosystem CIs in order for them to work. I've fixed the build scripts on our end, thanks again.

@dominikg
Copy link
Collaborator

dominikg commented May 9, 2023

i'm not exactly sure what is going on here but the yarn v1 workspace stuff seems to prevent the generated resolutions in workspace/quasar/quasar/package.json from working.

I can't be of much help here as i am not familiar with yarn v1 and won't start learning about it now. Please update the branch to latest main too (when i cloned it i got an outdated version with pnpm@7) This might be because you initially forked it before we updated it.

@yusufkandemir
Copy link
Contributor Author

@dominikg, it respects the resolutions and symlinks the nested modules. But, it probably gets confused due to the pnpm node_modules structure in the resolved module, creating faulty symlinks. This is an example symlink Yarn created: node_modules/vite/node_modules/esbuild -> ../../.pnpm/[email protected]/node_modules/esbuild (relative to workspace/quasar/quasar, resolves to non-existent path: workspace/.pnpm/...)

When I removed node_modules in workspace/vite/packages/vite and used yarn to install dependencies, then re-trigger the install process in workspace/quasar/quasar, it worked fine. It required two minimal changes in the Vite codebase to keep Yarn happy:

  • Adding "version": "0.0.0" to packages/vite/src/types/package.json
  • Adding "version": "0.0.0" to packages/vite/types/package.json

@dominikg
Copy link
Collaborator

This has not been an issue in any of the other tests in vite-ecoystem-ci and i'm not sure if i understand you correctly but you can't be suggesting to switch vite to yarn to solve this.

fyi @sapphi-red ^ the package.json in vite/types had been added by you.

@sapphi-red
Copy link
Member

I think it's fine with adding version field to those files.

But I'm curious about the reason of using yarn v1. Yarn v1 is already frozen and by using nodeLinker: 'node-modules' yarn v3 will work like v1 (i.e. without pnp).
https://yarnpkg.com/getting-started/migration#step-by-step

@yusufkandemir
Copy link
Contributor Author

@dominikg I am not suggesting Vite switch to Yarn, I shared my findings hoping they would help. Even if the version field is added, the ecosystem-ci script would need to use Yarn for Vite when cloning, installing, building, etc.

@sapphi-red because it works. We plan to migrate to pnpm and use workspaces for the whole repo. But, we are in the middle of significant changes such as migrating our code to ESM and supporting it in userland as well. For the same reason, we don't want to switch to Yarn v3 now, we'll move away from it anyway. I have just given Yarn v3 a try and it didn't work out of the box, some packages didn't resolve correctly, and the migration would take extra effort, which is not worth it at the moment.

If we don't have an easy solution to address the problem, I suggest running Quasar tests only with stable releases if possible, until we figure out a solution or migrate to pnpm.

@dominikg
Copy link
Collaborator

Even if the version field is added, the ecosystem-ci script would need to use Yarn for Vite when cloning, installing, building, etc.

This is not going to happen. vite repo uses pnpm and that's what we have to use for installation.

Curious how this works for you in vue-ecosystem-ci but not in vite?

@patak-dev
Copy link
Member

That's a good question. @sodatea do you know why Vue using pnpm wasn't an issue in vue-ecosystem-ci for Quasar? Linking the PR for reference vuejs/ecosystem-ci#7

@haoqunjiang
Copy link
Member

Because the vue ecosystem-ci uses a local registry, so the vue package there is not linked, but actually downloaded from the registry.
This setup makes local debugging a bit more difficult but avoids many edge cases.

@patak-dev
Copy link
Member

I wonder if it makes sense for ecosystem-ci to use the local registry as an opt-in only for some projects. As a flag passed to the test, localRegistry: true. Then we only pay for a degraded local experience on these projects.

@yusufkandemir
Copy link
Contributor Author

We have migrated from Yarn to pnpm: quasarframework/quasar#16990
pnpm run test quasar works fine now, so I think the PR is finally ready to merge.

@patak-dev
Copy link
Member

Great news @yusufkandemir! Let's merge this one and tried it out 👍🏼

@patak-dev patak-dev merged commit 6d2cbbd into vitejs:main Mar 19, 2024
1 check passed
@yusufkandemir yusufkandemir deleted the add-quasar-framework branch March 19, 2024 07:39
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.

6 participants