Skip to content

Load public svg resources by url - #230

Open
krisbreuker wants to merge 1 commit into
mainfrom
feature/svg-fetch-resource
Open

krisbreuker wants to merge 1 commit into
mainfrom
feature/svg-fetch-resource

Conversation

@krisbreuker

Copy link
Copy Markdown
Member

If an svg’s resource is set to a public module asset (mod::mymodule/public/image.svg), the svg is fetched directly client-side instead of being sent as a string. This way it can be cached by the browser if the same svg is used for multiple svg components.

Changes

  • Tollium <svg> component: LoadResource() now loads public module assets (mod::<module>/public/...) client-side from their /.wh/mod/<module>/public/... url instead of sending the SVG source inline, so the browser can cache it. The url gets a ?mtime=<unix timestamp> of the file's modification time, so a changed SVG is fetched again. Other resources, such as webdesign web/ folders, are still sent inline.

Testing

  • tollium.components.testsvg: two new steps.
  • Not covered by automated tests: a failed fetch (error logged, component left empty).

Deprecations and API changes

  • None

TODOs / FIXMEs

  • None

If an svg’s resource is set to a public module asset (`mod::mymodule/public/image.svg`), the svg is fetched directly client-side instead of being sent as a string. This way it can be cached by the browser if the same svg is used for multiple svg components.
@krisbreuker
krisbreuker requested a review from a team November 12, 2026 16:59
// Only fetch public module assets client-side; other resource urls (eg. webdesign web/ folders) aren't always served
STRING res_url;
IF (resource LIKE "mod::?*/public/*") {
res_url := GetModuleResourceURL(resource);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you're going to check for existence in the next line of code, but if that fails you'll still send a URL to a resource you now know will trigger a 404 (and triggers dynamic page work)

This branch has not been deployed

No deployments
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