chore: don't log more than 100 pages per route #11444
Conversation
🦋 Changeset detectedLatest commit: 6110473 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
| @@ -223,6 +223,9 @@ async function generatePage( | |||
| styles, | |||
| mod: pageModule, | |||
| }; | |||
|
|
|||
| const maxPathsToLog = 100; | |||
There was a problem hiding this comment.
You might want to disable this when verbose mode is enabled. I'm not 100% sure how you get it from here, but a search for verbose should hopefully find an example
There was a problem hiding this comment.
Idk, I found it useful when having some weird errors to log on each page and see what slug was failing so not showing them all would make this harder. Maybe a possibility would be to not log more than 100 by default and have an option to have the full log (using --verbose or equivalent). Don't forget the changeset!
|
This is "potentially" a breaking change because it changes a lot the DX for people. It could potentially be for the docs website, and for people that rely on this log for checking the progress of the build. (I do, in the Biome website) Also, I wouldn't want to use |
| const shouldTruncate = | ||
| pipeline.logger.options.level !== 'debug' && paths.length > maxPathsToLog; |
There was a problem hiding this comment.
Log all pages if when verbose mode is enabled
|
@ematipico do you think this shouldn;t be done at all, or that it should be in a major, or that there should be a specific config setting to enable or disable it? |
There was a problem hiding this comment.
+1 to the idea that knowing which page failed in a build is important. (In Starlight for example everything is "one route" but our docs has 4k pages so most would be hidden.)
But maybe that can be fixed somehow by always logging the current route when throwing an error? Not entirely sure if that's possible.
Otherwise a flag to disable the log truncation would help people without having to reach for the full --verbose output, which like Ema said, can be a bit overwhelming.
|
Sounds like this is a whole can of worms and not really worth the change! |
|
I think it's a valuable change, as you said logging can slow down the build and I agree 100%. I think we should evaluate this change for 5.0, and worth considering a new flag to log all pages. I know that Fred has strong feelings around logging, so we should make sure to run the suggestion through him |
|
I also agree logs can get very noisy about a certain threshold, but I'd want to see which page failed if possible during a build. If the current page could be bubbled into the error logs, I'd be +1 to this change! |
|
I'd lean on not changing this at all to prevent the risk of possibly not logging the current page if an error occurred. Error handling could be tricky and happen anywhere, so keeping the logging as simple as possible would be safest for me. |
|
Closing since we won't be doing this, I'm sure improvements could be considered outside of this, though. |
Changes
Currently we log every page generated, which is fine in most cases, but for very large sites is useless, makes the logs hard to use and can affect performance. This PR caps this to 100 pages per route, after which it displays a summary instead.
Testing
Most easily tested with the benchmark scripts.
Docs