search

fastify / fastify

Fast and low overhead web framework, for Node.js
https://www.fastify.dev
Other
31.43k stars 2.22k forks source link

Update modules for v5 #5116

Open jsumners opened 9 months ago

jsumners commented 9 months ago

To prepare for v5 we need to go through the list of repositories in this issue and:

  1. Determine if the module/project should be archived and marked as deprecated. Mostly we should look at the commit history to determine if the community is interested in the module/project. A secondary check would be inspecting the data for the module at npm-stat.com. Unless we have people using, and contributing to modules, we should be reducing our maintenance burden by archiving them.
  2. Evaluate if the module/project needs any updates. Most likely this will simply mean updating dependencies, dropping Node <= 16, and updating CI configuration accordingly (see footnote 1). Note: we must pin tap@16.3.9 for any projects utilizing it. See https://github.com/fastify/fastify/issues/5116#issuecomment-1790152300 for a required change related to this. Blue Oak is now OSI approved (https://opensource.org/license/blue-oak-model-license/), thus solving the licensing issue on this dependency.

If you are interested in helping with any of these, please create a new issue in the repo you're interested in that has the title "Updating for v5" and a body with something like "👋 I'm X, and would like to work on this." It should also include a link back to this issue. After that issue is created, a member of the Fastify team will create a next branch for you to target. Once that branch is created, you will be able to start a new Pull Request that targets it. We will help out with any questions you may have.

Footnote 1

Regarding updating the CI configuration in these projects. Most projects in the list should have a CI workflow that looks like https://github.com/fastify/fast-content-type-parse/blob/4de4e57f2d2c2893094c2a46069e04767b179d8e/.github/workflows/ci.yml#L1-L23. Thus, we have a few possible changes:

  1. If a project has a different configuration that the one linked in the previous paragraph, it is likely that the configuration should be update to match it.
  2. If the project is using the linked configuration, it should be updated to reference the v4 tag of the worklows repo once issue https://github.com/fastify/workflows/issues/106 has been resolved.
  3. If it is determined that the shared configuration should not be used for the project, the configuration should be updated as outlined at the top of this issue.
Uzlopak commented 9 months ago

@mcollina

Did you get feedback from openjs regarding tap v18 used as dev dependency?

Fdawgs commented 9 months ago

Majority of the repos are using the shared workflows so CI changes should be easy enough.

I'll hop on that.

eliphazb commented 8 months ago

@jsumners I've just created a issue to work on fastify/rate-limit plugin the link : https://github.com/fastify/fastify-rate-limit/issues/333

gurgunday commented 8 months ago

Note: we must pin tap@16.3.9 for any projects utilizing it.

Should we pin specifically to tap@16.3.9 or is ^16 sufficient?

gurgunday commented 8 months ago

Also, maybe we can also work on centralizing all CI workflows (and possibly most scripts) during this release cycle

I see that some repos use plugins-ci, which is great, but others use dedicated workflows

WDYT?

CC @Eomm

jsumners commented 8 months ago

Note: we must pin tap@16.3.9 for any projects utilizing it.

Should we pin specifically to tap@16.3.9 or is ^16 sufficient?

16.3.9 is the last release in the accepted release line. It should be pinned explicitly as an indicator of intention

Eomm commented 8 months ago

Also, maybe we can also work on centralizing all CI workflows (and possibly most scripts) during this release cycle

We have centralized all the main extranal software such as redis. so I think it is a matter of missing PR or unsupported versions

Right now the releases are going to change: we are waiting the OpenJS Foundation as npm provenance signature host

Fdawgs commented 8 months ago

As part of pinning tap, Dependabot configs also needs to be updated to ignore major tap versions to stop it from opening PRs, see: https://github.com/fastify/fastify/blob/396b8b95bf0f1313b9deeb387d68fdc7ffa97abe/.github/dependabot.yml#L35-L37

Eomm commented 8 months ago

A reminder that we have this notion page to see all the public repos and download stats:

nrayburn-tech commented 8 months ago

Just wanted to put a checklist of items to look at when updating a package that I have found so far. These won't apply for every repo, but they should be checked if they apply or not for every repo. I'll keep adding to this as I find anything else. (Feel free to edit/update this comment or copy it into the main issue summary.)

Fdawgs commented 8 months ago

We may not need to pin tap: https://github.com/openjs-foundation/cross-project-council/issues/1170#issuecomment-1802733531

jsumners commented 8 months ago

I am keeping up with that thread (as you probably can see). I will update the task list here if it comes about in time that Blue Oak is authorized. But we can bump tap at any time without affecting the major-ness of any releases.

Uzlopak commented 7 months ago

Maybe we should archive the docs-chinese, docs-portuguese and docs-korean, as there is basically no activity.

mcollina commented 7 months ago

We can not upgrade tap in any repo that supports node v14, so it'd be a Fastify v5 affair.

As far as it seems, Blue Oak is on track to be OSI approved in a month. If it's not, then we would need to migrate (seems very unlikely).

mcollina commented 7 months ago

I think it's mostly safe for v5 to upgrade to latest tap.

gurgunday commented 6 months ago

fastify-dx is archived, it can be marked as done

Fdawgs commented 6 months ago

As far as it seems, Blue Oak is on track to be OSI approved in a month. If it's not, then we would need to migrate (seems very unlikely).

@mcollina I remember seeing you mention a Linux/OpenJS Foundation mailing list discussing this but lost the link. I think the consensus was it's good to go?

climba03003 commented 6 months ago

I remember seeing you mention a Linux/OpenJS Foundation mailing list discussing this but lost the link.

Starting from http://lists.opensource.org/pipermail/license-review_lists.opensource.org/2023-November/005441.html The last comment was Dec 27 2023, I do not subscribe the mailing list. I don't know if there were new comment in Jan 2024.

jsumners commented 6 months ago

There is not yet a resolution on the OSI inclusion of Blue Oak.

jsumners commented 6 months ago

fastify-dx is archived, it can be marked as done

@galvez please mark fastify-dx as deprecated on npmjs.com.

jsumners commented 5 months ago

Blue Oak is now OSI approved. We should be able to utilize tap@18 without issue at this point (unless @mcollina says we need even further approval from OpenJSF)`.

https://opensource.org/license/blue-oak-model-license/

Fdawgs commented 5 months ago

Blue Oak is now OSI approved. We should be able to utilize tap@18 without issue at this point (unless @mcollina says we need even further approval from OpenJSF)`.

https://opensource.org/license/blue-oak-model-license/

Will sort the .gitignore files for all the repos, see https://github.com/fastify/skeleton/issues/51#issuecomment-1915323188

climba03003 commented 5 months ago

Tracking for OpenJSF approval https://github.com/openjs-foundation/cross-project-council/issues/1170

jsumners commented 5 months ago

It is approved. My updates above are accurate.

https://github.com/openjs-foundation/cross-project-council/pull/1245

Fdawgs commented 5 months ago

Will sort the .gitignore files for all the repos, see fastify/skeleton#51 (comment)

PR made for all repos which sorts this.

simoneb commented 4 months ago

Hi all, we (Nearform) would like to help out with this, even take care of all the repos as a batch. Before taking this up, I would like to confirm that the plan is still as described in this issue:

  1. assessing whether the repo is to be archived/deprecated
  2. if not, make the necessary changes
    1. update CI
    2. bump deps

On the second point above, I would like to confirm that the changes we see in one of the repos that were already updated are those that should be done across the other repos, for example: https://github.com/fastify/fastify-cookie/pull/276. Meaning:

Thanks

jsumners commented 4 months ago

Yes, that is an accurate summation. Regarding dependencies, please update to the latest major versions unless there is some reason to prevent it (e.g. the module changed to ESM only).

Fdawgs commented 4 months ago

Blue Oak is now OSI approved. We should be able to utilize tap@18 without issue at this point (unless @mcollina says we need even further approval from OpenJSF)`.

https://opensource.org/license/blue-oak-model-license/

Any that have already been updated to pin tap need to be updated to unpin it.

jsumners commented 4 months ago

That is complicated. tap@16 isn't going to give us full coverage checking on Node 20+, but current tap is a very heavy dependency. Switching to node:test has other issues. I really don't know what to do here.

gurgunday commented 4 months ago

I think long term node:test looks amazing and for me is delightful to use, but I agree that it's too much work right now

simoneb commented 4 months ago

I agree that node:test is the way to go in the future, but I wouldn't see it as part of this issue.

climba03003 commented 4 months ago

If we plan to change the test runner, it should be the last step before release. The result is that, it create a large conflict between current main and next branch.

mcollina commented 4 months ago

I don't think we should change the test runner, we already have enough work as it is.

synapse commented 4 months ago

Hey guys, we've come across some issues with tap when updating it to the latest versions.

What would be the best scenario in these case? Tap 16.x seems to be running just fine for now with all the other deps upgrades. Should we keep version 16?

jsumners commented 4 months ago

Most of the changes for tap@18 are covered in https://github.com/fastify/fastify/pull/4978. But, as I said, this specific dependency is complicated. The old tap@16 uses istanbul for code coverage and there are known issues with it not picking up coverage on Node >= 20 (which is why you see c8 in the changes linked in this response). I don't like the idea of changing the test framework, but I'm not convinced it is avoidable.

jsumners commented 4 months ago

@fastify/collaborators please read https://github.com/orgs/fastify/discussions/39

mcollina commented 4 months ago

Should we keep version 16?

No, not really, we should move to v18 or move to node:test. The former is preferred because it'll be less changes.

synapse commented 4 months ago

Hey everyone, us, the team at Nearform has been doing an amazing job over the past week to upgrade as many plugins as possible to be ready for Fastify v5. We've also put together a comprehensive list of all the plugins, along with additional links to the PRs we've opened and NPM Stats for repositories that we believe are no longer in use or not worth upgrading. However, the decision on whether to upgrade these repositories is ultimately up to you.

:x: - closed without merge :+1: - done by us + merged :white_check_mark: - done by somebody else :stopwatch: - done by us - not merged yet :skull_and_crossbones: - we consider that this repo should be archived (up to you) :toolbox: - working on this :question: - doubts/questions :warning: - has issues upgrading

gurgunday commented 4 months ago

This is absolutely amazing, especially since I also had some ideas about archiving a few of the repos

Thank you so much

gurgunday commented 4 months ago

@synapse small issue with the list showing dead packages, some packages were checked against the wrong names

For instance we should check for @fastify/accept-negotiator instead, which is downloaded 1 million times a week, and not 0

These (and potentially others) seem to be wrong: https://www.npmjs.com/package/accept-negotiator https://www.npmjs.com/package/action-pr-title https://www.npmjs.com/package/github-action-merge-dependabot

I can't edit comments so can't fix it manually, would be appreciated if you could take a look

synapse commented 3 months ago

Hey @gurgunday sorry for that, I've rechecked all repos previously marked as to be archived. Indeed the NPM naming was different from the repo name. I've updated them, fortunately there were only a few of them off. I also left you the markdown source there in case you want to update the main one. Let me know if there are thing that seem off

Fdawgs commented 3 months ago

Ah crud, just realised we missed something. We need to update the plugin version across all of them, for example: https://github.com/fastify/under-pressure/blob/7eca099076d525dea037405582f0cd48ee593cf3/index.js#L312

gurgunday commented 3 months ago

I think someone said doing that without first releasing a version of v5 causes a crash?

Haven't looked at the code though so not sure

nrayburn-tech commented 3 months ago

The fastify plugin seems to support multiple versions by using Semver ranges. https://github.com/fastify/fastify-plugin?tab=readme-ov-file#fastify-version

Somebody from the fastify team should probably share what is expected, but I would guess something like the below.

Once the next branch is updated, if it only supports v5 then that branch won’t be usable until a v5 version is released. It might make sense to wait for an alpha or beta version of v5 that can be used before making that change. Or, doing an alpha release solely for this reason.

Fdawgs commented 3 months ago

I think someone said doing that without first releasing a version of v5 causes a crash?

Ah yeah, good point. Ignore me 😹

gurgunday commented 3 months ago

@nrayburn-tech yeah next plugins will definitely only support v5 (and node LTS versions >=18) in general — that's been the case in previous cycles

We should release an alpha/beta very soon, as soon as we can rebase next

Then we (or @Fdawgs, who is good at this 😄) can run a script to mass update next plugins to change that plugin version

puskin94 commented 3 months ago

it looks all the plugins / repos are in one of this 2 statuses:

  1. a PR for reaching v5 has been submitted and merged
  2. a PR for reaching v5 has been submitted and it is in the review status

v5 is near 🥳

simoneb commented 3 months ago

Thanks everybody. I'd like to mention especially @synapse, @bilalshareef, @puskin94 and @gesma94 who have contributed this work on behalf of Nearform.

simoneb commented 2 months ago

Is there anything else that needs support to get this over the line?

gurgunday commented 2 months ago

Other than a few small issues in repos like restartable and autoload, we're nearly done with plugin updates

I will propose archiving a few plugins, so I haven't updated them, but the rest is complete I think

We should start doing prereleases of v5 soon