Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

SteadEXE
Copy link
Contributor

@SteadEXE SteadEXE commented Feb 18, 2025

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
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
Copy link
Contributor Author

SteadEXE commented Feb 19, 2025

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 ?

@@ -80,19 +80,6 @@ describe('Fastify adapter', () => {
});
});

describe('when trying to get a non-existing file', () => {
Copy link
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
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
1 check passed
@kamilmysliwiec
Copy link
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