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

perf: avoid computing paths on each request #15318

Merged
merged 2 commits into from Dec 12, 2023

Conversation

patak-dev
Copy link
Member

Description

We should optimize responding from the server as much as possible. For large projects, the time it takes the server to serve already transformed requests will greatly affect reloading time.

This PR avoids normalize paths and path checks that were done on each request. It is safe to cache this in the middleware constructor after #15166

I also moved the warning to another function because it isn't important for the main logic of the handler.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Copy link

stackblitz bot commented Dec 11, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-dev patak-dev added the performance Performance related enhancement label Dec 11, 2023

// check if public dir is inside root dir
const publicDir = normalizePath(server.config.publicDir)
const rootDir = normalizePath(server.config.root)
Copy link
Member

Choose a reason for hiding this comment

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

const resolvedRoot = normalizePath(
config.root ? path.resolve(config.root) : process.cwd(),
)

root: resolvedRoot,

config.root is already normalized, so we don't need to normalize it here.
I think we can normalize the publicDir when we resolve the config as well.
const resolvedPublicDir =
publicDir !== false && publicDir !== ''
? path.resolve(
resolvedRoot,
typeof publicDir === 'string' ? publicDir : 'public',
)
: ''

Copy link
Member Author

Choose a reason for hiding this comment

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

Done with config.root. About publicDir, we got hit by it not being normalized here #15317, so I think it is a good idea. But I was thinking we should do this in the next minor or major because some plugins may expect it to not be normalized. Hopefully there aren't many instances like that but better to play safe.

I think that all the directory paths in the resolved config should be normalized and without a trailing slash: root, publicDir, outDir. We should check what we are doing with base. There are a lot of places were we are doing a path.posix.join or similar when we could be directly concatenating the strings if we trust the resolved config.

@patak-dev patak-dev merged commit 0506812 into main Dec 12, 2023
15 checks passed
@patak-dev patak-dev deleted the perf/avoid-computing-paths-on-each-request branch December 12, 2023 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants