Skip to content

fix(fastify): serve index file when it exists#1587

Merged
kamilmysliwiec merged 4 commits into
nestjs:masterfrom
SteadEXE:master
Feb 20, 2025
Merged

fix(fastify): serve index file when it exists#1587
kamilmysliwiec merged 4 commits into
nestjs:masterfrom
SteadEXE:master

Conversation

@SteadEXE

@SteadEXE SteadEXE commented Feb 18, 2025

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

It sends index file when this file does not exists.
When it does it replies with a 404 not found.

Issue Number: #1583 1583

What is the new behavior?

It sends the index file when file exists.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@SteadEXE

Copy link
Copy Markdown
Contributor Author

I removed the 404 e2e test as code was trying to serve index.html anyway.
Maybe this behavior can be inhibited with an option, but that is out of the scope of this PR.

@SteadEXE

SteadEXE commented Feb 19, 2025

Copy link
Copy Markdown
Contributor Author

Sorry to ping @kamilmysliwiec but is it possible to take a look at it, we would like to migrate to Nest v11 without relying on patch-package ?

});
});

describe('when trying to get a non-existing file', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should remain the default behavior unless a flag (TBD which flag) is enabled. For express it is fallthrough: true so I guess we could use it for Fastify too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, I think it's a good idea, I just made a commit to re-use this flag on fastify, I also updated the tests, it seems to cover all the cases. Tell me what you think

@kamilmysliwiec kamilmysliwiec merged commit 54b3e4f into nestjs:master Feb 20, 2025
@kamilmysliwiec

Copy link
Copy Markdown
Member

LGTM

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