Flow: Overriding vaadin component versions with pnpm enabled

[rokkolesa]

Description of the bug

Sometimes there may be some bugs (possibly breaking ones) in some shipped components. An example was a bug in vaadin-overlay (vaadin/vaadin-overlay#176), which caused an incorrect behaviour of Dialogs when multiple instances were closed at once.

This was since fixed and new patch versions were released (3.2.20, 3.3.2, 3.4.1 and 3.5.1). However, the shipped vaadin-dialog version in 14.2.0 is 2.3.0, which still uses 3.3.1 version of vaadin-overlay (the broken one) and I cannot override the version it uses when using pnpm. I could however force the usage of another version when using npm.

Things I tried (note that I always deleted pnpm-lock.yaml, package.json and node-modules and ran prepare-frontend and build-frontend via vaadin-maven-plugin - unless otherwise stated):

1. Include the annotation

@NpmPackage(
        value = "@vaadin/vaadin-overlay",
        version = "3.3.2"
)

somewhere on my UI class. This caused the package.json to change the dependency to this version, but the pnpm-lock.yaml still locked the 3.3.1 version. I went and explored and it seems that the version had changed in both vaadin.dependencies as well as dependencies of package.json! This caused the VerisonsJsonConverter class to think that the version of vaadin-overlay is not user-managed, when in turn it is. The versions.json file then included the "@vaadin/vaadin-overlay": "3.3.1" line and pnpmfile.js locks 3.3.1. This is also a bug I believe - in creating package.json which in turn causes some unwanted side effects.

1. When the first fix didn't work, I tried to just correct the package.json file myself (I did not delete it afterwards :)) - not via an annotation. I simply corrected the vaadin-overlay version from 3.3.1 to 3.3.2 (I actually added it) in dependencies. This caused pnpm to lock the correct version. However, things still didn't work. The problem was that vaadin-dialog (along with other components) specified [email protected] as it's dependency. I didn't use vaadin-overlay in my end-project and it wasn't picked up by webpack when bundling (or serving when using webpack-dev-server). Therefore, version 3.3.1 was still being used by vaadin-dialog and the whole application. I think this is due to the nature of pnpm.

2. Using the fix in my second attempt I now tried to use OverlayElement somewhere in my application to try and force the 3.3.2 version to be bundled. This actually caused both versions to be bundled and in turn break the application with an error Failed to execute 'define' on 'CustomElementRegistry': the name "vaadin-overlay" has already been used with this registry. This is actually expected at this point because I was using two versions of vaadin-overlay.

3. My last attempt was to not include the annotation or manually changing package.json, but by modifying pnpmfile.js. I included the lines:

  if (dependencies["@vaadin/vaadin-overlay"]) {
    dependencies["@vaadin/vaadin-overlay"] = "3.3.2";
  }

This seems to be the correct fix, as I found no traces of version 3.3.1 in pnpm-lock.yaml after running pnpm i --shamefully-hoist manually. However, I cannot test this correctly as the pnpmfile.js gets fully overridden if I run maven plugin or if I run the application in development mode. So it would be great if we could modify this file just like we can modify webpack.config.js. (EDIT: I actually tested it afterwards - after obtainng the pnpm-lock.yaml with the modified pnpmfile.js I just ran the maven plugin.. the bundled version was 3.3.2 and there were no traces of 3.3.1, so it seemed to be working!)

Are there some other ways to make this kind of override? It seems that the nature of pnpm is 'a dependency will always use their own package.json to resolve the necessary versions.. if you want to override this, use pnpmfile.js'.. so it makes sense to give us the ability to modify it as well. Or maybe you can come up with a better fix? :) I also believe that I should be able to do the override given that it is possible using npm.

Versions

- Vaadin / Flow version: `14.2.0`/ `2.2.0`
- Java version: `11`
- OS version: `macOS 10.15.4`

[pleku]

@rokkolesa there was an issue with the release of the vaadin-overlay package to npm and the LATEST tag was left to 3.2.20. This caused failures https://github.com/vaadin/flow/issues/8448 are you sure you just didn't experience this too ? It should be now fixed.

We have noticed also that there is a regression starting from version 4.6.0, even though we are locking the versions in pnpmfile.js it is not removing duplicates from pnpm-lock.yaml https://github.com/vaadin/flow/issues/8434 so you might have been seeing that too.

With pnpm and Vaadin, it should be possible to override the version by just adding the dependency as a to top level dependency to package.json.

Please check @rokkolesa if the issue is already fixed, or try by installing pnpm 4.5.0 in your system ? Or then you can wait for 14.2.1 (c. next week).

[rokkolesa]

@rokkolesa there was an issue with the release of the vaadin-overlay package to npm and the LATEST tag was left to 3.2.20. This caused failures #8448 are you sure you just didn't experience this too ? It should be now fixed.

I didn't see any references to 3.2.20 anywhere - neither in pnpm-lock.yaml nor in the generated bundle, so I don't think I had this problem.

We have noticed also that there is a regression starting from version 4.6.0, even though we are locking the versions in pnpmfile.js it is not removing duplicates from pnpm-lock.yaml #8434 so you might have been seeing that too

The referenced issue says that the workaround is to delete both node_modules as well as pnpm-lock.yaml. I always did that when testing. Besides, my 4th attempt actually does the same as in the example in the referenced issue - force locks the vaadin-overlay version using the same code, which worked in the end. But I cannot use that as a fix because pnpmfile.js gets rewritten every time.

With pnpm and Vaadin, it should be possible to override the version by just adding the dependency as a to top level dependency to package.json.

Yeah, that's what I figured.. but it doesn't seem to work like that now.. unless I'm doing something wrong..

Please check @rokkolesa if the issue is already fixed, or try by installing pnpm 4.5.0 in your system ? Or then you can wait for 14.2.1 (c. next week).

I was running pnpm version 4.14.4 so I will try downgrading and try again and report here.

I can wait for 14.2.1 as we aren't using this in production yet - we just started the migration process. So I have no problem with waiting a bit more :) But this can be a problem in the future..

[rokkolesa]

Okay I have tested on downgraded pnpm to 4.5.0 and there was no difference from 4.14.4.

To summarize:

1. Added the annotation: it doesn't matter if it's there or not.. the lockfile is exactly the same with or without it.
2. Add "@vaadin/vaadin-overlay": "3.3.2" manually to dependencies of package.json of the end project: lockfile now locks 3.3.2 for end project, but still locks 3.3.1 in dependencies (because [email protected] specifies 3.3.1). The bundled version is still 3.3.1.
3. The same as 2, but import OverlayElement in one JS file: both versions bundled, application breaks - duplicate webcomponent registration.
4. Leave package.json as it is but only force 3.3.2 via pnpmfile.js: lockfile now has correct version for all occurrences of @vaadin/vaadin-overlay.. only 3.3.2 is bundled and application works with 3.3.2 fixing the bug with vaadin-dialogs. EDIT: This is not a solution or a good enough workaround as the pnpmfile.js is rewritten everytime.

[pleku]

Thanks for the details. It seems that the version overriding we have done for pnpm is not then working for peer dependencies. Not sure if it will be straight forward to fix it, or if we need to enable users to have a custom pnpmfile.js where they can force specific versions of things.

[rokkolesa]

If I'm not mistaken, this issue is again present on vaadin-flow 2.3.1. It seems that VersionsJsonFilter does not have the else clause in getFilteredVersions method: https://github.com/vaadin/flow/blob/2.3.1/flow-server/src/main/java/com/vaadin/flow/server/frontend/VersionsJsonFilter.java#L46 whereas the fix was just that: https://github.com/vaadin/flow/pull/8545/files

[pleku]

@rokkolesa thanks for the heads up - sorry it has taken a while to get back to this after all the notifications after summer vacations... but you're right the 2.3. branch is missing the fix for some reason even though https://github.com/vaadin/flow/commit/8055c67c222aca067e767c3991a04c72fb8f7d5f shows that it should be in 2.3 which is not the case. Not sure how this has happened but I'll backport the fix to 2.3 too