Add baseURL support for fontface assets#1275
Merged
Conversation
Fontface assets were the only asset type not benefiting from baseURL, making font path resolution inconsistent with images, audio, etc. The loader now strips any url() wrapper before prepending baseURL, so both plain paths and url()-wrapped paths work correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds baseURL support for fontface assets so font paths resolve consistently with other loader asset types, and updates examples/docs accordingly.
Changes:
- Include
fontfacein wildcardsetBaseURL("*", ...)configuration. - Strip a
url(...)wrapper fromfontfacesrcbefore applyingbaseURL. - Update JSDoc/examples/changelog and add tests for the new behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/melonjs/src/loader/loader.js | Adds fontface to wildcard baseURL and strips url(...) before baseURL concatenation in load() |
| packages/melonjs/src/loader/parsers/fontface.js | Adjusts fontface parser example and wraps non-url( sources into url(...) |
| packages/melonjs/tests/loader.spec.js | Adds assertions for wildcard baseURL including fontface and a new url() stripping test |
| packages/melonjs/CHANGELOG.md | Documents fontface baseURL support |
| packages/examples/src/examples/ui/ExampleUI.tsx | Updates example fontface asset to use a plain path instead of url(...) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
140
to
144
| baseURL["js"] = url; | ||
| baseURL["tmx"] = url; | ||
| baseURL["tsx"] = url; | ||
| // XXX ? | ||
| //baseURL["fontface"] = url; | ||
| baseURL["fontface"] = url; | ||
| } |
Comment on lines
+517
to
+522
| if ( | ||
| asset.type === "fontface" && | ||
| typeof asset.src === "string" && | ||
| asset.src.startsWith("url(") | ||
| ) { | ||
| asset.src = asset.src.slice(4, -1); |
Comment on lines
+21
to
+23
| // FontFace constructor expects src in `url(...)` format | ||
| if (!data.src.startsWith("url(")) { | ||
| data.src = "url(" + data.src + ")"; |
Comment on lines
+273
to
+299
| it("should strip url() wrapper from fontface src before applying baseURL", () => { | ||
| loader.setBaseURL("fontface", "assets/"); | ||
|
|
||
| // simulate what load() does: strip url() then prepend baseURL | ||
| const asset1 = { | ||
| name: "font1", | ||
| type: "fontface", | ||
| src: "url(font/test.woff2)", | ||
| }; | ||
| const asset2 = { name: "font2", type: "fontface", src: "font/test.woff2" }; | ||
|
|
||
| // strip url() wrapper for fontface assets | ||
| if (asset1.src.startsWith("url(")) { | ||
| asset1.src = asset1.src.slice(4, -1); | ||
| } | ||
| if (asset2.src.startsWith("url(")) { | ||
| asset2.src = asset2.src.slice(4, -1); | ||
| } | ||
|
|
||
| // apply baseURL | ||
| asset1.src = loader.baseURL[asset1.type] + asset1.src; | ||
| asset2.src = loader.baseURL[asset2.type] + asset2.src; | ||
|
|
||
| // both should resolve to the same path | ||
| expect(asset1.src).toBe("assets/font/test.woff2"); | ||
| expect(asset2.src).toBe("assets/font/test.woff2"); | ||
|
|
- Fix url() stripping to handle quotes and whitespace via regex - Preserve local() font source descriptors (don't wrap in url()) - Skip baseURL prepend for local() and data: sources - Update JSDoc to list fontface as a supported type for setBaseURL - Rewrite test to use stubbed parser instead of duplicating logic - Add test for local() passthrough Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
baseURL, resolving font paths consistently with all other asset types (image, audio, json, etc.)url()wrapper before prependingbaseURL, so bothfont/foo.woff2andurl(font/foo.woff2)work correctlyurl()wrappersTest plan
fontfaceis included in wildcardsetBaseURL("*")url()stripping before baseURL is applied🤖 Generated with Claude Code