Skip to content

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

@bolt-new-by-stackblitz
Copy link

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
@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.

3 participants