fix: add undeclared dependencies and resolve others (fix #8286, fix #7581)

[merceyz]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Documentation
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

The PR fulfills these requirements:

• It's submitted to the dev branch (or v[X] branch)
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
• It's been tested on a Cordova (iOS, Android) app
• It's been tested on a Electron app
• Any necessary documentation has been added or updated in the docs (for faster update click on "Suggest an edit on GitHub" at bottom of page) or explained in the PR's description.

Other information:

Fixes #8286
Fixes #7581
Ref quasarframework/quasar-starter-kit#159

In order to not rely on unpredictable hoisting this PR makes a few changes.

• require.resolve Webpack loaders
  See https://github.com/yarnpkg/berry/tree/b605eaf5785dc91d0f5248da67ec9f34460900ca/packages/yarnpkg-doctor#no-unqualified-webpack-config

• Declare dependencies
  See https://yarnpkg.com/advanced/rulebook#packages-should-only-ever-require-what-they-formally-list-in-their-dependencies

• types in tsconfig.json
  Using the types option in tsconfig.json disables the default behaviour of loading all visible types in the global scope, this PR avoids using it and instead uses a triple-slash directive to avoid disabling that behaviour
  https://www.typescriptlang.org/tsconfig#types
  https://www.typescriptlang.org/docs/handbook/triple-slash-directives.html#-reference-types-

• Making vue and vue-router peer dependencies
  vue is currently a dependency of @quasar/app while it's undeclared in quasar and the starter-kit, this is problematic as it relies on hoisting to place the correct vue version in an accessible location for all of them and any other dependency that might need access to it. Hoisting is an implementation detail and does not guarantee this to happen.
  For example when using workspaces one workspace might use vue@2 while the other uses vue@3, which one getting hoisted to the root is undefined.
  To solve this vue is changed to be a peer dependency which correctly reflects the intent that one workspace (quasar project) uses one version of vue

[IlCallo]

Hi @merceyz and thanks for the contribution, can you clarify some aspects of this PR?

In particular:

• on a conceptual level, Quasar wants to remove from devs the hassle of resolving deps conflicts and managing updates of commonly used dependencies, providing all those deps from @quasar/app package with pinned versions.
  This PR and the related one on the started kit break this concept and would be a pretty big change for Quasar project
• it's not entirely clear why usingrequire.resolve is needed either
• you also removed "types": ["quasar"] from the TS config preset, ignoring the comment up there
• on the starter kit PR you added a /// <reference types="@quasar/app" /> which AFAIK isn't needed, with no explanation whatsoever

You marked this as not breaking change, but it seems to be, at least at a conceptual level

[merceyz]

Sorry about that, I should have opened it as a draft. I've updated the PR to explain what's going on.

[IlCallo]

About require.resolve Webpack loaders: if I understood correctly, this change should only apply to @quasar/app webpack chain additions, as the scaffolded project isn't meant to be a library and be imported from other projects, thus not requiring its usage.

As far as I can understand, the docs examples showing how to extend the webpack chain into the project quasar.conf.js shouldn't need the require.resolve usage, nor it's needed into the scaffolded .eslintrc.js.

On the other hand, it's correct to use it into AE docs and put there a note about using require.resolve, since those are meant to be added as dependencies.

Am I wrong on these assumptions?

[IlCallo]

Btw, I'm still not sure about this, as it happened quite some times to rely on hoisting when testing new packages versions before updating it into @quasar/app: adding the dependency at project level overwrites the one provided by default and allow people to easily try out new versions without setting up the whole quasar codebase

I guess we can obtain a similar result using yarn resolutions, but I'm not sure it would work the same

[merceyz]

About require.resolve Webpack loaders: if I understood correctly, this change should only apply to @quasar/app webpack chain additions, as the scaffolded project isn't meant to be a library and be imported from other projects, thus not requiring its usage.

Correct.

As far as I can understand, the docs examples showing how to extend the webpack chain into the project quasar.conf.js shouldn't need the require.resolve usage

I replaced all instances I found though those can probably be reverted but doesn't hurt to keep it.

nor it's needed into the scaffolded .eslintrc.js.

If you're referring to quasarframework/quasar-starter-kit@113dacc then it is actually, vue-eslint-parser needs access to it which requires an absolute path
https://github.com/vuejs/vue-eslint-parser/blob/e69dd815502c44a722bc2e10de87050c06cc4f08/src/script/index.ts#L531

[IlCallo]

Just a little update: we didn't forget about this PR, we're discussing about this

[rstoenescu]

Hi,

I appreciate the contribution. However, this is a "no-go" for several reasons:

1. We cannot ensure a great development experience if some of current q/app's deps are not pinned. We've had a ton of problems in the past due to vue/vue-loader/vue-router introducing breaking changes in minor or patch releases. Or there were simply regressions introduced -- and we cannot rely (or enforce) on Vue team fixing something asap. The engine needs to go on without breaking mid-flight. This is why some packages are direct deps of q/app. Furthermore, not shipping versions of tested vue packages (by us) makes things much more complex than they need to be and require a minimal level of knowledge from the developers using Quasar (like how to deal with package.json, how npm and yarn actually works and commands for upgrading stuff for it). Not everyone knows this (and testament to this are quite a few Github tickets). We want at least 99% of devs to experience Quasar as "it just works" or "not breaking mid-flight". Run a command and focus on your app rather than on the boilerplating around it.
2. Making the changes here in this PR means we're gonna do a major breaking change for all Quasar CLI projects. Every developer would need to figure out which deps he needs to install and which version. It's not only "vue" that needs to be installed. It's @vue/compiler-sfc and @vue/server-renderer too (which were not extracted in this PR and this will 100% break apps when versions for all these 3 packages do not match). But the point is: devs upgrade Quasar CLI for the next minor version and they are also required to read our release notes otherwise their apps won't work. Only a few read the release notes. So frustration++ for most devs. Plus the tickets that will be logged by people having some weird / unofficial setups that will complain.
3. We've tested setups using workspaces with Yarn v1 and there's no problems (or problems were fixed). Yes, Yarn v2 has issues and the required changes would actually force us to drop certainty whether apps will work or not... each day would bring up one question in the morning: "does my project still works?". So, for now, Yarn v2 is not something that we want to support. We have plans on a CLI using Vite under the hood, and this might be the way forward where Yarn v2 will also be tackled.
4. Some of the things in the PR are not right (typing in random order of thoughts coming to my mind):

• rather than calls directly to require.resolve() (which would break other use-cases, for example while developing and using sym-links) it would have been better to use the set of helpers in lib/helpers/get-*
• vue-router is declared as optional; it's not optional, it's mandatory
• new deps added to q/app related to babel (core / loader / etc) would need changes to @quasar/babel-preset-app otherwise devs who need to make changes to this package would need to know they have to touch q/app's package.json babel deps as well... not to mention testing the changes themselves would be hell (as you won't know for sure which deps you're running)
• marking ui/package.json with a dependency (like core-js) is definitely a no-go; Quasar UI does not import or do anything with code from core-js -- it's just the build system that may do it, so why add extra friction points with npm/yarn on package hoisting

There are other things that need a note here, but this post is getting ridiculously long.
Again, not trying to say "this PR is not appreciated". Quite the contrary. It's just not something that we can tackle at the moment.