fix(TS): missing ssrContext param for language set

[Evertvdw]

When setting a language in SSR mode a ssrContext parameter should be passed: https://next.quasar.dev/options/quasar-language-packs#dynamical-ssr-

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

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch and not the master 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.

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

[IlCallo]

LGTM, have you tested the autocomplete works correctly both in a SSR and not-SSR project?

[rstoenescu]

Isn't this breaking the this.$q.lang.set() usage? That requires only one param.

[Evertvdw]

@IlCallo How do you usually test this? What I did now was modify this file in my running projects node_modules, but that does not feel like the best way...

[IlCallo]

@Evertvdw I have my own fork of the quasar repo locally, I do the changes there, then create the test projects anew and run yarn add ../<quasar folder>/app.

If the problem is TS related, I always restart VSCode too.

In this case the test would be to check that the second parameter is required only if SSR mode is enabled, so you'll need to test it on a new project and check the parameter isn't there. Then add SSR mode, restart VSCode and check the second parameter is now required. Then try the use case (providing there the ssrContext parameter obtained from the boot options object) and check everything works fine (eg. that it builds correctly both in SPA and SSR mode).

@rstoenescu as far as I can tell from the code and the docs, the second parameter is required when in SSR mode.
This PR changes the method signature to require the second parameter only when SSR mode is added.
This notifies the dev that he should provide a second parameter. Since the ssrContext provided to the boot function will be defined only when building for SSR, an undefined will be passed over when building for SPA, making everything work correctly.

Do you see any flaws in this plan?

[rstoenescu]

Like I said, this.$q.lang.set() requires only one param, regardless of Quasar mode being used.

[Evertvdw]

As far as I can see in the source code it requires 2 arguments: https://github.com/quasarframework/quasar/blob/vue3-work/ui/src/lang.js#L29
But the 2nd argument is only required when you have SSR enabled. Only downside is that when you want to use $q.lang.set() on the client side when you have SSR enabled you will have to pass a random second argument. We can make the second argument optional of course, but on SSR side it is not optional and will throw an error when you do not pass it :)

[IlCallo]

Yeah, that's the code I was talking about.
Maybe you mean that this.$q.lang.set() and Quasar.lang.set() behave differently?

[Evertvdw]

Anything you need from me to get this PR along?

[IlCallo]

Here's the explanation: all methods which require ssrContext already have it managed internally when used via this.$q
But Quasar globals act as "singletons" so they don't have the SSR context, nor should they.

At this point, we'll need to separate the types in some way.
GlobalQuasarIconSet must be changed as well, as it was based on the same wrong assumption.

We need something like a QSingletonGlobals interface which must then be improrted here

quasar/ui/build/build.types.js

Line 368 in e9a84de

writeLine(contents, `import { ${ injectionName }, BaseQGlobals } from "./globals";`)

instead of BaseQGlobals and used here

quasar/ui/build/build.types.js

Line 397 in e9a84de

writeLine(contents, 'export const Quasar: { install: (app: App, options: Partial<QuasarPluginOptions>) => any } & BaseQGlobals')

@Evertvdw are you up for the task?

[Evertvdw]

I don't think so. If you have clear what needs to be done I think you can better do it. I don't have that much experience with Typescript.

Edit I gave it a try anyways, is this ok?

[IlCallo]

LGTM, thanks!
If you confirm it works for your use case, I'll squash & merge this, since other use-case won't be affected by this change at this point

[Evertvdw]

@IlCallo yes I already confirmed it works!

[IlCallo]

Thanks, will be in next beta release :)

[Evertvdw]

@IlCallo This change has not been merged in Quasar v1 but that is also necessary as I get this error there also now :)

[IlCallo]

Merged into dev branch too, will be out with next release
Thanks for the heads up!