Skip to content

Lift up checks to notAcceptable to the middleware#434

Merged
dahlia merged 11 commits intofedify-dev:nextfrom
ThisIsMissEm:refactor/lift-up-not-acceptable
Sep 21, 2025
Merged

Lift up checks to notAcceptable to the middleware#434
dahlia merged 11 commits intofedify-dev:nextfrom
ThisIsMissEm:refactor/lift-up-not-acceptable

Conversation

@ThisIsMissEm
Copy link
Contributor

@ThisIsMissEm ThisIsMissEm commented Sep 13, 2025

Summary

Following discussion in discourse, this PR shows the triggering of notAcceptable happening from the middleware before any of the dispatchers are triggered, since they should only be triggered if they would actually be acceptable and lead to a response.

This means that when you're passing through requests via content-negotiation, the fedify dispatcher is not fired if it's not a Accept content-type that fedify would actually handle.

This would be a major breaking change, hence targeting next

Related Issue

There doesn't seem to be related issues reported, however, it's a common point of confusion for integraters "why is my fedify dispatcher firing for this html request?"

Changes

List the specific modifications made in this PR.
Focus on what was changed without going into detail about impact.

  • Lifted check for Accept header content-type up to middleware
  • Moved acceptsJsonLd to negotiation.ts (from handle.ts)
  • Removed old notAcceptable tests from handler.test.ts — I'm still not 100% sure how these tests were working.

Benefits

Describe the advantages or improvements brought by these changes.
Explain how these changes affect the project, users, or performance.

Checklist

  • Did you add a changelog entry to the CHANGES.md?
  • Did you write some relevant docs about this change (if it's a new feature)?
  • Did you write a regression test to reproduce the bug (if it's a bug fix)?
  • Did you write some tests for this change (if it's a new feature)?
  • Did you run deno task test-all on your machine?

Additional Notes

Include any other information, context, or considerations.

@ThisIsMissEm ThisIsMissEm force-pushed the refactor/lift-up-not-acceptable branch from 65b31be to 92104c4 Compare September 13, 2025 19:55
@ThisIsMissEm

This comment was marked as outdated.

@ThisIsMissEm ThisIsMissEm force-pushed the refactor/lift-up-not-acceptable branch from 92104c4 to f6468c9 Compare September 20, 2025 22:07
@ThisIsMissEm

This comment was marked as outdated.

@ThisIsMissEm ThisIsMissEm marked this pull request as ready for review September 21, 2025 01:47
@ThisIsMissEm
Copy link
Contributor Author

A bunch of these changes can be pulled out (e.g., deno caching and the duplicate key in deno.json, and silencing installs — I was trying to get cleaner more focused test logs, as currently they're a little too big to debug & cause rendering issues (like the lines fail to render as you scroll)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2025

The docs for this pull request have been published:

https://dd4fe93e.fedify.pages.dev

@github-actions
Copy link
Contributor

The latest push to this pull request has been published to JSR and npm as a pre-release:

Package Version JSR npm
@fedify/fedify 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/cli 2.0.0-pr.434.1662+ae9fb437 JSR
@fedify/amqp 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/cfworkers 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/denokv 2.0.0-pr.434.1662+ae9fb437 JSR
@fedify/elysia 2.0.0-pr.434.1662+ae9fb437 npm
@fedify/express 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/h3 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/hono 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/nestjs 2.0.0-pr.434.1662+ae9fb437 npm
@fedify/next 2.0.0-pr.434.1662+ae9fb437 npm
@fedify/postgres 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/redis 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/sqlite 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/sveltekit 2.0.0-pr.434.1662+ae9fb437 JSR npm
@fedify/testing 2.0.0-pr.434.1662+ae9fb437 JSR npm

Copy link
Member

@dahlia dahlia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! Thanks for your contribution.

@dahlia dahlia merged commit 35bf253 into fedify-dev:next Sep 21, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants