Bump typescript from 4.0.5 to 4.1.2

[dependabot]

Bumps typescript from 4.0.5 to 4.1.2.

Release notes

Sourced from typescript's releases.

TypeScript 4.1

For release notes, check out the release announcement.

For the complete list of fixed issues, check out the

• fixed issues query for TypeScript v4.1.0 (Beta).
• fixed issues query for TypeScript v4.1.1 (RC).
• fixed issues query for TypeScript v4.1.2 (Stable).

Downloads are available on:

• npm
• Visual Studio 2017/2019 (Select new version in project options)
• NuGet package

TypeScript 4.1 RC

For release notes, check out the release announcement.

For the complete list of fixed issues, check out the

• fixed issues query for TypeScript v4.1.0 (Beta).
• fixed issues query for TypeScript v4.1.1 (RC).

Downloads are available on:

• npm
• Visual Studio 2017/2019 (Select new version in project options)
• NuGet package

TypeScript 4.1 Beta

For release notes, check out the release announcement.

For the complete list of fixed issues, check out the

• fixed issues query for TypeScript v4.1.0 (Beta).

Downloads are available on:

• npm
• Visual Studio 2017/2019 (Select new version in project options)
• NuGet package

Commits

• f1e53da Update LKG
• ca766df Fix/jsx global preferred over config implicit (#41476) (#41583)
• d72de70 Actually install playwright in artifact build script now that it's not instal...
• ba1b9d2 Bump version to 4.1.2 and LKG
• c12cc53 Remove bundledPackageName from release 4.1 (#41536)
• 612c19d Merge pull request #41500 from weswigham/pick-crash-fix
• 1f27a71 Comment position feedback
• 3307cff Fix crash on attempting to suggest a ts import for a synthetic js resolution
• abf4b63 Update LKG
• 8d249ea Merge remote-tracking branch 'origin/master' into release-4.1
• Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
• @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

[Eomm]

@airhorns we should remove the package-lock form the repo as in all other plugins in the org.

I read your point #4 (comment) here but we should not develop with a defined package-lock but always with the last one and updating it is a huge effort

[airhorns]

I mean ... dependabot does it for you? That doesn't really seem like a lot of effort to me and I still think it's worth having contributors on the same page. Have you ever experienced the heisenbug where your tests pass locally but not in production because CI is installing a different version of the dependencies than you have locally? NPM and yarn both invested so much in giving us the tools to avoid that, why not use them?

[airhorns]

And wouldn't it be better to have discrete PRs like this one that show when and why things break? Not have that just happen when dependencies release?

[airhorns]

@mcollina I have a fix for this but I don't have push rights to this repo. Is that something you folks grant for plugin repos like this or should I PR it?

inspector ~/C/fastify-passport (dependabot/npm_and_yarn/typescript-4.1.2) ➜  git po
ERROR: Permission to fastify/fastify-passport.git denied to airhorns.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

[mcollina]

Go ahead and send a PR for now

[Eomm]

I mean ... dependabot does it for you? That doesn't really seem like a lot of effort to me and I still think it's worth having contributors on the same page. Have you ever experienced the heisenbug where your tests pass locally but not in production because CI is installing a different version of the dependencies than you have locally? NPM and yarn both invested so much in giving us the tools to avoid that, why not use them?

heisenbug happens for application, not for modules: CI run with a fresh install and not with a package-lock, so a failing CI could not mean that your local repo is breaking and this is a real heisenbug.
it is an effort that adds noise to commit history and PRs tho

In this repo it details this aspect and to be brief package-lock should be committed for application (and CI run npm ci not npm install etc)

[dependabot]

OK, I won't notify you again about this release, but will get in touch when a new version is available.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

[airhorns]

@Eomm I am genuinely not trying to be a stick in the mud -- I just don't understand why less determinism could possibly be better. I searched that repo you linked and only found the package-lock.json mentioned once, and that was about including it or excluding it in the published files, not about if it should be used in a development workflow. Maybe I missed the part where it talked about that?

heisenbug happens for application, not for modules

Why couldn't it happen for modules? This module has transitive dependencies and all the same opportunities for problems that an application could have where folks working on this thing could have different packages installed locally than what CI uses. CI should indeed run npm ci, but without a package lock, it is going to re-resolve and install a potentially different dependency tree than what developers are working with locally. Is that somehow desirable for a reason I am missing? Why would you want to open up to the opportunity of CI failing and your local tests passing?

I also think that PR noise is really not that big a deal because dependabot just does everything. Do you feel that extra commits are really that burden some compared to extra human time spent debugging version mismatches between environments? Cause I'd rather have a longer history and trust you and I are in the same environment when discussing a bug. The only reason this PR didn't get auto-merged is specifically because it needed human intervention to fix, and I think that being surfaced is a good thing so that consumers of this package don't need to spend their time hunting down the issue themselves. I'd rather computers do the computer-y work of figuring out what breaks when rather than burdening the community with it when it can be automated for free. Again, if I am just totally out to lunch here say so. I also have experience in some other package ecosystems where everyone uses Gemfile.lock everywhere or goes even farther and vendors dependencies right into repos like Go.

[Eomm]

Maybe I missed the part where it talked about that?

Sorry, I was referring to this PR that has not been merged yet:
nodejs/package-maintenance#302

To recap briefly: the package-lock is not published for a module, so every user that installs the package will resolve the dep tree and npm will install all the latest version of the dependencies - based on the semver ranges in the package.json of this plugin.

heisenbug happens for application, not for modules

I misspelt, I wanted to say the opposite:
Let's assume fastify-secure-session will release a bug that affects this module and a user find this issue.
We would tag that issue as cant-reproduce till dependabot open a PR and we merge it - that is not in real-time.
I meant this use case for "heisenbug" in modules.
So the lock would be an obstacle in this case, or not useful if it is necessary to run npm install every time

In applications, you must always be aligned to the whole team so the package-lock is absolutely necessary - and the whole team must use the same npm version too.

I also think that PR noise is really not that big a deal because dependabot just does everything

I agree that a bot could do the dirty job but for example, it opens other questions to me:

• more PR: noise
• harder to check PR: you must check the package-lock if someone commit it or ask to don't commit it
• why in the package-lock there is typescript 4.1.2 and in the package json 4.0.5? Since 4.1.2 has requested a change, would it not necessary to update the package.json as well?
• different settings between org projects will increase the effort that each individual must put in the project*

Is that somehow desirable for a reason I am missing?

From my point of view, CI for application should never fail -thanks to package-lock; but CI for modules should fail as fast as it can since users will download a fresh new dependencies tree, so CI should simulate to be a user.

Again, if I am just totally out to lunch here say so

Maybe I don't get it this sentence, but for sure I would like to convince you, not force you at all!
I'm here to share and discuss 😄

Regarding other kinds of locks, I'm totally unfamiliar

* notice I'm not a member of the TS team, so this discussion will not "impact" me, since I don't have the knowledge to support and maintain a TS module as you do 👍
I'm just discussing to share what I learnt and maybe learn something new as well!

[airhorns]

To recap briefly: the package-lock is not published for a module, so every user that installs the package will resolve the dep tree and npm will install all the latest version of the dependencies - based on the semver ranges in the package.json of this plugin.

Yup

Let's assume fastify-secure-session will release a bug that affects this module and a user find this issue. We would tag that issue as cant-reproduce till dependabot open a PR and we merge it - that is not in real-time.
So the lock would be an obstacle in this case, or not useful if it is necessary to run npm install every time

Don't you think dependabot would be faster than a user in the community discovering the issue, debugging it, deciding to open an issue, and causing the humans here to have to spend time triaging? It's free to run nightly and I don't think this repo would blow through the github actions minute limit for open source projects.

Also, using a package-lock.json and dependabot doesn't mean the process you described couldn't happen. People could still report issues and we could still debug them. If you want to try out a specific version of a package to test if something is broken, you'd either have to manually install it, or modify package.json to install it. Either way, you're touching something, I don't think one is really faster than the other. Either way, you have to run some npm command to set up your local package state, so I don't get why package-lock.json would be that much slower?

And then finally, even if we want to use the process you described as the main way, dependabot still acts as a layer of defense, doesn't it? If a dependabot pr gets opened and is red, then the immediate action we could take as maintainers is to limit the supported version range to exclude the new version that is broken. Again, the computers automate the discovery of that issue instead of burdening the community with it. Is the perceived simplicity of the dev workflow more important than automating access to that information to you? Cause I would say the workflow is equivalently simple, and if there's one option that generates more free information, I want that one.

more PR: noise

It's not noise if it's red -- it's a broken dependency or transitive dependency. We have to action that by limiting the version range or by fixing the bug. It's annoying, I will grant you that, but it's still real users and bugs experienced by real users saved. The green PRs are automerged, so that's notifications ya, but I'd rather optimize for user happiness than a few extra notifications.

harder to check PR: you must check the package-lock if someone commit it or ask to don't commit it

npm ci does that automatically

why in the package-lock there is typescript 4.1.2 and in the package json 4.0.5? Since 4.1.2 has requested a change, would it not necessary to update the package.json as well?

Because the package can support different versions of dependencies, that's why we don't use exact versions in the package and instead use ~ version. We explicitly don't want to lock down what the package supports if it supports both. I just want to lock down what developers work on so we're not debugging environment mismatches between developers.

different settings between org projects will increase the effort that each individual must put in the project*

I think other packages should have package-lock.json for the same reasons, that's why I brought it up. I think a lot of packages pre-date support for package-lock.json or it isn't clear to folks why it would be nice. If it's a fastify org policy then so be it, but again, I feel like I must be missing something as to why that would be the case.

From my point of view, CI for application should never fail -thanks to package-lock; but CI for modules should fail as fast as it can since users will download a fresh new dependencies tree, so CI should simulate to be a user.

The main branch CI going red randomly seems worse to me than a dependabot branch's CI going red with the exact change that caused it in the PR's diff. A red main branch means new releases are unsafe -- that's bad if you ask me and that state should be avoided as much as possible.

Also ... in that PR you linked, it says this:

The package-lock.json will get created automatically by default and it will not be published into the registry, i.e. it is only intended to be used for development purposes. npm recommends you commit this file into your source control.

I didn't read that as meaning they only meant that for applications -- do you read it that way?

[mcollina]

(@airhorns you should have admin access here, please check).

On the debate about package-lock, I completely agree with sindresorhus/ama#479 (comment). This has shaped a significant part of the fastify ecosystem.

Some of the repo use the lockfiles because some deps do not really follow semver, making it impossible to develop anything otherwise. Typescript is one of those deps (there are many others). Generically if a dep does not really follow semver, either we drop it or we had lock files to the repos.

@airhorns developed a good chunk of this repo and I think we should keep the lockfile if he thinks this will be better.

[Eomm]

Don't you think dependabot would be faster than a user in the community discovering the issue

Yes, I think you are right. My doubt is about opened PR for very deep modules in the tree: I think it is not scalable in terms of notification received that could hide more important. GH is working on this for these kinds of setup I think right now.

npm ci does that automatically

If I'm not wrong, the npm version should be the same between you and the contributor - I had trouble with old npm major version really (from 4 to 5 mainly). So it could make a bit annoying the process

I just want to lock down what developers work

I get it and it makes totally sense. As said I'm not a TS user so I have overseen this use case that is good to know 👍

I feel like I must be missing something as to why that would be the case.

Right now I think it is only a matter of:

• reduce notification/noise (I agree a red PR is not noise - but 80/20 wins sometimes: we can't fix broken deep dependencies in any case, unless sticks all the tree as root one in case of trouble)
• reduce work (automation has been introduced some weeks ago, so it was a manual operation).
• speed up PR (personally I feel I should check locally a package-lock change over a PR with only code)
• don't stick node/npm version on dev machine

The main branch CI going red randomly seems worse to me than a dependabot branch's CI going red

I can't say "no", this happened for fastify-helmet tho
The dev dependencies are a lot more than dependencies that "matters", so it could lead to turning off those notifications.
I think that if OSS is not your job, we should find a good trade-off to work effectively without too much burden.

I didn't read that as meaning they only meant that for applications -- do you read it that way?

I remember a good discussion about it, but I can't find it - with more details. Sorry for that, if I will catch it (serendipity) I will link it!

Writing down these comments, I realized that the "issue" is my business:
I do OSS in my free time and I don't want low-value notifications (in my opinion) than my actual job.
It must be a little funny - as setting up bots, new feature etc..
I weighed the pro/cons and I think a less noise is more valuable than one less maintainer cause the PR-chaos would generate in me (maybe I click to much often on the "watch" button 😄 )

developed a good chunk of this repo and I think we should keep the lockfile if he thinks this will be better.

I totally agree, if you don't think this setup is annoying, you must keep it.

With or without the package-lock the module will be fine in any case thanks to the people who take care of it.
This is what really matters

PS: I read the ci.yml workflow:

fastify-passport/.github/workflows/ci.yml

Line 30 in a2114e0

run: npm install --frozen-lockfile

and I don't know the --frozen-lockfile flag and I was not able to find it in the npm docs is this correct?

[airhorns]

Ah oops that's a leftover from switching back from yarn to npm, I shall fix.

Thanks for the good points. I'll think about it some more and read some more, I think consistency with the rest of the community packages to make sharing contributors easy is valuable as well.

[mcollina]

The current approach come from heavy-duty maintainers. I receive ~200 github notifications a day. A project with package-lock generate 2-3x the amount of one without. With the latest bit of automation, things lands automatically, but there is the occasional CI slip and a manual land.

We added it to fastify-next because we could not keep the CI green, and it was less work than dealing with the notifications.

[fox1t]

Here I agree with @Eomm and @mcollina. I can't get any benefit from having a lockfile in a project that is not an application. Lockfiles are not committed to npm anyway and npm/yarn resolves versions and dependencies at install time. Even if we have version A locked, the user might end up using version 'B'. It is way better to just rely on npm install to be sure that the project is still compatible with the last versions of its dependencies.

On the other hand, I think that dependabot could be useful during major version changes

[airhorns]

I can't get any benefit from having a lockfile in a project that is not an application. Lockfiles are not committed to npm anyway and npm/yarn resolves versions and dependencies at install time.

Do you not agree that having developers and CI locked to the same versions is valuable?

It is way better to just rely on npm install to be sure that the project is still compatible with the last versions of its dependencies.

Dependabot creates this same situation by bumping dependencies to their latest versions as they come out, but with it tracked and with red PRs if it doesn't work instead of random, untraceable red master CI runs.

If the issue is PR noise, I was going to suggest we switch dependabot to opening only one PR with all the changes but I am not sure that's supported yet dependabot/dependabot-core#1190

[mcollina]

I'm totally fine with whatever you folks want to do with this module - I never wrote a line of code for it - I'm a true believer that in OSS whom does the work should make the decisions. I'm an hard -1 in Fastify, avvio, and most of the modules I work regularly.

[fox1t]

Do you not agree that having developers and CI locked to the same versions is valuable?

Not using a lock file on CI is actually valuable for projects that are not applications: using npm i warns you if your package is broken before publishing it to the npm registry.
Let's recap the two cases.

Using the lock file:
0. developer A adds a new code and commits

1. developer A uses a lock file
2. CI uses npm ci and the lockfile
3. there is no update to the dependencies since they are locked
4. the package is published to npm
5. the user of the package can't use the same lock file (since they are not committed to the npm registry) and uses npm I to install it
6. npm resolves the dependencies and installs different versions
7. the package is broken, and the user has to report it

Without using the lock files:
0. developer A develops a new code

1. the code is committed and pushed
2. the CI uses npm i to install a version that respects semver, but that breaks the package itself
3. CI fails, and the developer can look into it before merging and publishing it

That's exactly the point of not using lock files for packages published to npm.

I agree with you, @mcollina, but I think it should be nice to have consistency if a package is inside the Fastify organization, even more, when it is objectively a better option.

@airhorns sorry to point this out again and being so pedantic, but I just want to be sure that we are on the same page. If we are, and you still prefer using lock files in packages published to the registry, I am fine with that! :)

[airhorns]

@airhorns sorry to point this out again and being so pedantic, but I just want to be sure that we are on the same page. If we are, and you still prefer using lock files in packages published to the registry, I am fine with that! :)

No problem at all, I am not super passionate about this, I just want to make sure I am not taking crazy pills.

Using the lock file:

This describes the way things worked before Dependabot and automerging were a thing. I don't think it works like that anymore, because dependabot will update the lockfile to install the same versions that npm would for a user. The master branch of this repo is guaranteed to have the newest version of dependencies in the lock file if those dependencies pass the test, or for there to be a failing PR with exactly the version bump that causes a failure opened by Dependabot. For free.

Removing the lock file also doesn't solve the problem of developers using different dependencies than users -- if you ask me, it actually makes it worse. Developers would have to constantly remember to re-install fresh dependencies to match what users are getting, whereas a lockfile makes it explicit that they need to install new dependencies as the locked versions are changed by Dependabot.

0. developer A develops a new code
   the code is committed and pushed
   the CI uses npm i to install a version that respects semver, but that breaks the package itself
   CI fails, and the developer can look into it before merging and publishing it

Does that not seem like a terrible developer experience for the contributor? The developer's local tests could pass fine if they had an outdated dependency which CI installed a newer version of. Debugging why CI is failing, especially if your tests are passing locally, seems annoying, and a lock file rules out dependency version mismatches as a cause. Worse, without a lockfile, if the developer just ran npm install, they wouldn't even get the newer version of the dependency because the existing one they had locally would satisfy the version range. They'd have to rm -rf node_modules; npm install to reset their environment to behave the same way as CI. And they'd have to do that repeatedly over time to keep up. This is exactly the problem that lockfiles were designed to solve.

Because of Dependabot, the lockfile doesn't prevent awareness of newer, broken versions like it used to. We get automated lockfile upgrades, information about which upgrades might have broken things, and consistency between all contributors and CI. I think the notifications noise argument is a good one that makes it about it being worth it or not, and for open source heroes like @mcollina I totally buy that it is not worth it.

I know that the lockfile doesn't impact what users of the package install, I am just talking about the developer experience. Lockfiles solve the problem of coordinating what dependencies are installed on different systems, and regardless of if you're working on an application or a package, you have a bunch of different systems in the mix that should be coordinated if you ask me.

[mcollina]

I agree with you, @mcollina, but I think it should be nice to have consistency if a package is inside the Fastify organization, even more, when it is objectively a better option.

We already have it for certain packages, e.g. fastify-next.

[fox1t]

@airhorns OK now I fully understood: I was more on the user's side of the story and you are on the developer/contributor one. :)