vaadin / platform

Vaadin platform 10+ is a Java web development platform based on Vaadin web components. If you don't know to which repository your bug report should be filed, use this and we'll move it to the right one.
https://vaadin.com
515 stars 76 forks source link

Do not pin Polymer version for npm #495

Closed web-padawan closed 5 years ago

web-padawan commented 5 years ago

Pinning Polymer in planform package.json results in 2 versions being installed:

npm i @vaadin/vaadin
$ npm ls|grep @polymer/polymer|sed "s/.*@poly/@poly/"|sort|uniq -c
   1 @polymer/polymer@3.0.5
   2 @polymer/polymer@3.0.5 deduped
   1 @polymer/polymer@3.1.0
  47 @polymer/polymer@3.1.0 deduped

We should not pin Polymer version for npm, any minor version should be ok.

Legioth commented 5 years ago

any minor version should be ok

This assumes there's a 0% risk of accidentally breaking changes between minor (or maintenance) versions. This is obviously not the case since there are humans involved in the equation.

web-padawan commented 5 years ago

Other option would be to generate npm-shrinkwrap.json. However, please note:

It’s strongly discouraged for library authors to publish this file, since that would prevent end users from having control over transitive dependency updates.

In the front-end ecosystem, the npm dependencies generally specify "required minimal version" (for us it is ^3.0.0), and then assume the user will take care of the proper version pinning.

Legioth commented 5 years ago

assume the user will take care of the proper version pinning

That approach assumes that the user has knowledge of potentially breaking changes in all libraries that they're directly or indirectly using. My experience is that this assumption is a quite common thing to complain about by developers who think JavaScript isn't a mature programming language.

One of the values of the Vaadin platform is that we take care of such concerns on the users' behalf. Would it make sense to offer a separate vaadin-shrinkwrap module that users can choose to include in their project if they want?

ripla commented 5 years ago

From earlier discussions, the way to go is to include package-lock.json in our starters and specify the minimum versions as suggested by @web-padawan. We should also make sure the release builds retain the package-lock.json so we can reproduce a platform build if needed.

web-padawan commented 5 years ago

We should also make sure the release builds retain the package-lock.json so we can reproduce a platform build if needed.

If we want to publish a lockfile to npm, we must use npm-shrinkwrap.json instead of package-lock.json. Attaching the example created for vaadin-core 12.0 branch:

https://gist.github.com/web-padawan/2cb378492432f20e4167e5c183572125

Note this will prevent our users from upgrading to Polymer 3.1.0 and enforce using 3.0.5 I feel we would end up with this eventually when adding npm support in Flow.

web-padawan commented 5 years ago

Created a small demo, you can check it out:

npm install web-padawan/npm-shrinkwrap-demo#master --save

Note that there would be still duplicates if user installs Polymer 3.1.0 himself:

screen shot 2019-01-14 at 14 23 52

So, by depending on this kind of package, our users will have to delegate us the right to manage their frontend dependencies, and agree not to try updating them manually.

ripla commented 5 years ago

Decided to: 1) Unpin Polymer and specify a minimum version instead with ^3.0.0 2) Create an NPM shrinkwrap for vaadin and vaadin-core when releasing 3) Separately release @vaadin/vaadin, @vaadin/vaadin-shrinkwrap, @vaadin/vaadin-core and @vaadin/vaadin-core-shrinkwrap 4) Update repository readme with information about NPM releases

ripla commented 5 years ago

Closed in favour of https://github.com/vaadin/platform/issues/515