Skip to content

Conversation

@evnchn
Copy link
Collaborator

@evnchn evnchn commented Nov 23, 2025

Motivation

While working on #5493 I also noticed that, full cached, NiceGUI documentation reaches out to:

  • index.html (obviously)
  • codehilite.css

The latter we should be able to get rid of?

Implementation

  • Filename may change so we do ${this.filename} with the accompanying props.
  • Compute the file name and return the response using _generate_codehilite_css(), which is shared.
  • (Questionable) Apply @lru_cache(maxsize=1) to _generate_codehilite_css() because it doesn't ever change in runtime (an asusmption, is this right?)

Progress

  • I chose a meaningful title that completes the sentence: "If applied, this PR will..."
  • The implementation is complete.
  • Pytests have been added (or are not necessary).
  • Documentation has been added (or is not necessary).

Results

Slow 4G no CPU throttling (because network is the bottleneck)

Before: 1.46s load
After: 1.00s load

@evnchn evnchn added feature Type/scope: New or intentionally changed behavior review Status: PR is open and needs review labels Nov 23, 2025
@falkoschindler falkoschindler added this to the 3.4 milestone Nov 24, 2025
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Thanks, @evnchn!

It took a while until I understood the need for the hash. First I thought we only need this single line:

headers={'Cache-Control': core.app.config.cache_control_directives},

But we need a way to change the content of this file in upcoming NiceGUI versions, so the URL shouldn't be fixed.

(Would it be possible/reasonable to move it into /_nicegui/<version>/?)

Anyway, the PR looks good as it is. We can think about further optimizations later.

@evnchn
Copy link
Collaborator Author

evnchn commented Dec 1, 2025

Would it be possible/reasonable to move it into /_nicegui//

Probably not, since we don't know what combo of library the user has installed (could be super out of date or extremely recent)

Yeah let's leave this PR as it is for now...

@falkoschindler falkoschindler added this pull request to the merge queue Dec 1, 2025
Merged via the queue into zauberzeug:main with commit 93b6553 Dec 1, 2025
8 checks passed
@evnchn evnchn deleted the codehilite-fast branch December 1, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Type/scope: New or intentionally changed behavior review Status: PR is open and needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants