🔥 feat: Add Support for Serving Embedded Static Files#3448
Conversation
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughA new method, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Router
participant FSHandler
User->>App: Call StaticFilesystem(prefix, filesystem, config)
App->>Router: registerStaticFS(prefix, filesystem, config)
Router->>FSHandler: Setup handler with config
User->>App: HTTP GET /static/file
App->>FSHandler: Serve file from fs.FS
FSHandler-->>User: Return file content or next middleware
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
app.go (1)
17-18: Unnecessary extra alias creates import-group churn
app.goimports the sameio/fspackage thatrouter.goalready brings in (aliased asiofs). Having two different aliases (fsvsiofs) across the project makes grepping harder and quickly leads to style drift.-import ( - … - "io/fs" +import ( + … + "io/fs" // TODO: decide on **one** project-wide alias, e.g. “iofs” or plain “fs”router.go (4)
10-11: Choose a single alias forio/fsThe package is imported here as
iofsbut as plainfsinapp.go. Pick one form to improve readability and keep IDE-based import grouping stable.
448-565: Large copy–paste block duplicatesregisterStatic; extract common helper
registerStaticFSis ~120 lines that are almost byte-for-byte identical toregisterStatic, differing only in:
- the source of the files (
FSvsRoot)- the default
Rootvalue (".")- the parameter types
Duplicating this logic multiplies maintenance cost (bug fixes, new flags, security tweaks). Consider extracting the shared parts into a single unexported helper that accepts an
*fasthttp.FSalready pre-configured, e.g.:func (app *App) mountStaticHandler(prefix string, fsConf *fasthttp.FS, cfg ...Static)Both
registerStaticandregisterStaticFSwould then build thefasthttp.FSstruct (differing in only two fields) and delegate the rest.Benefits:
- One canonical path-rewrite algorithm
- One place to toggle new
Staticoptions- Smaller diff-footprint for future features
468-478:Root: "."is ignored whenFSis set – remove to avoid confusionAccording to
fasthttp.FSdocs, when theFSfield is non-nil theRootfield is ignored. LeavingRoot: "."in place is harmless but can confuse future readers into thinking the path is relevant.- FS: filesystem, - Root: ".", + FS: filesystem,
500-517: Nil-pointer guard for caller mistakes
registerStaticFSdereferencesconfig[0]multiple times without a length check after the first guard.
If the caller passes an empty slice and later iterations of the code are changed, this could regress.Adding a short nil-safe accessor avoids the hazard:
var userCfg Static if len(config) > 0 { userCfg = config[0] }and replace every
config[0].FoowithuserCfg.Foo.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app.go(2 hunks)router.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app.go (1)
router.go (2)
App(207-229)Router(23-47)
| // StaticFilesystem Supports Serving Static Resources from an Embed.FS File System | ||
| // | ||
| // The filesystem parameter can directly accept an embed.FS instance. | ||
| func (app *App) StaticFilesystem(prefix string, filesystem fs.FS, config ...Static) Router { | ||
| app.registerStaticFS(prefix, filesystem, config...) | ||
| return app | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Expose the new helper through the public Router interface
StaticFilesystem is now public on *App, but the Router interface (defined in router.go, lines 22-47) hasn’t been extended.
As a result:
var r fiber.Router = app // common pattern
r.StaticFilesystem("/assets", myFS) // ← does **not** compile
The omission also means *Group cannot forward the call, limiting feature parity with Static.
type Router interface {
…
Static(prefix, root string, config ...Static) Router
+ StaticFilesystem(prefix string, filesystem fs.FS, config ...Static) Router
…
}After extending the interface, forwarders for *Group can be added with two lines:
func (g *Group) StaticFilesystem(p string, fs fs.FS, cfg ...Static) Router {
return g.app.StaticFilesystem(getGroupPath(g.Prefix, p), fs, cfg...)
}🏁 Script executed:
#!/bin/bash
set -e
# Locate the Router interface declaration and its methods
rg "type Router interface" -A 20
# Confirm if StaticFilesystem is already declared in router.go
rg "StaticFilesystem" -n router.go || echo "No StaticFilesystem in router.go"
# Inspect Group type and existing Static methods
rg "type Group" -A 10
rg "func .*Group\).*Static" -nLength of output: 1560
Expose StaticFilesystem on the Router interface and Group
The new StaticFilesystem helper is public on *App but isn’t declared on the Router interface (router.go) or forwarded by *Group (group.go), preventing common patterns like:
var r fiber.Router = app
r.StaticFilesystem("/assets", myFS) // does not compilePlease add:
• router.go (within type Router interface):
type Router interface {
…
Static(prefix, root string, config ...Static) Router
+ StaticFilesystem(prefix string, filesystem fs.FS, config ...Static) Router
…
}• group.go (alongside func (grp *Group) Static):
func (grp *Group) StaticFilesystem(p string, fs fs.FS, cfg ...Static) Router {
return grp.app.StaticFilesystem(getGroupPath(grp.Prefix, p), fs, cfg...)
}| // Don't execute middleware if Next returns true | ||
| if len(config) != 0 && config[0].Next != nil && config[0].Next(c) { | ||
| return c.Next() | ||
| } | ||
| // Serve file | ||
| fileHandler(c.fasthttp) | ||
| // Sets the response Content-Disposition header to attachment if the Download option is true | ||
| if len(config) > 0 && config[0].Download { | ||
| c.Attachment() | ||
| } | ||
| // Return request if found and not forbidden | ||
| status := c.fasthttp.Response.StatusCode() | ||
| if status != StatusNotFound && status != StatusForbidden { | ||
| if len(cacheControlValue) > 0 { | ||
| c.fasthttp.Response.Header.Set(HeaderCacheControl, cacheControlValue) | ||
| } | ||
| if modifyResponse != nil { | ||
| return modifyResponse(c) | ||
| } | ||
| return nil | ||
| } | ||
| // Reset response to default | ||
| c.fasthttp.SetContentType("") // Issue #420 | ||
| c.fasthttp.Response.SetStatusCode(StatusOK) | ||
| c.fasthttp.Response.SetBodyString("") | ||
| // Next middleware | ||
| return c.Next() | ||
| } |
There was a problem hiding this comment.
Missing nil-filesystem validation
If a caller accidentally passes a nil fs.FS, the fast-http handler panics the first time it tries to open a file.
if filesystem == nil {
panic("StaticFilesystem: filesystem cannot be nil")
}Fail fast with a clear message rather than a runtime panic deep inside fasthttp.
| use: true, | ||
| root: isRoot, | ||
| path: prefix, | ||
| // Public data | ||
| Method: MethodGet, | ||
| Path: prefix, | ||
| Handlers: []Handler{handler}, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Route metadata sets use: true but only registers GET/HEAD
use: true traditionally means match all HTTP methods.
Because you later add the route only to GET & HEAD stacks, the flag is misleading for downstream tools that rely on it (e.g. route printers, metrics).
Set use: false or register the handler for all methods, mirroring the behaviour of app.Static.
|
Thx for the effort Is everything go 1.17 ready? V2 is supporting this old version |
|
@miseelu can you provide tests for this feature |
|
@miseelu just so you know, i'm currently trying to reduce the custom methods for adding routes in this issue the idea behind this is not to create the route struct in different places |
|
@miseelu Please provide Unit-Tests and update the Documentation |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new StaticFilesystem method to serve embedded static files from an embed.FS, aligning with Fiber’s existing static file API.
- Introduces
registerStaticFSinrouter.goto handle embed.FS file serving - Adds
StaticFilesystemwrapper inapp.go - Keeps backward compatibility with the original
Staticmethod
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| router.go | Implements registerStaticFS with path rewriting and handler registration for embedded FS |
| app.go | Exposes StaticFilesystem on App to call registerStaticFS |
Comments suppressed due to low confidence (3)
router.go:448
- [nitpick] Consider adding unit tests for
registerStaticFSto cover path rewriting logic and embedded FS handling to ensure correct behavior.
func (app *App) registerStaticFS(prefix string, filesystem iofs.FS, config ...Static) {
router.go:476
- The time package is not imported, causing a build error. Add
import "time"at the top of the file.
CacheDuration: 10 * time.Second,
router.go:560
- The sync/atomic package is not imported, causing a build error. Add
import "sync/atomic"at the top of the file.
atomic.AddUint32(&app.handlersCount, 1)
| // StaticFilesystem Supports Serving Static Resources from an Embed.FS File System | ||
| // | ||
| // The filesystem parameter can directly accept an embed.FS instance. |
There was a problem hiding this comment.
[nitpick] The doc comment should follow Go conventions and include usage examples. For example:
// StaticFilesystem serves embedded static files from an fs.FS.
// Example:
// app.StaticFilesystem("/assets", embeddedFiles)| // StaticFilesystem Supports Serving Static Resources from an Embed.FS File System | |
| // | |
| // The filesystem parameter can directly accept an embed.FS instance. | |
| // StaticFilesystem serves static resources from an fs.FS, such as an embed.FS instance. | |
| // | |
| // Example: | |
| // // Embed static files using embed.FS | |
| // //go:embed static/* | |
| // var embeddedFiles embed.FS | |
| // | |
| // app.StaticFilesystem("/static", embeddedFiles) |
| } | ||
|
|
||
| // Create route metadata without pointer | ||
| route := Route{ |
There was a problem hiding this comment.
|
@miseelu can you check our last hints |
|
@miseelu I think you should tag your PR with “Work In Progress”, after all I don't think the changes you're making now will fit into the branch. |
gaby
changed the title
|
@ReneWerner87 Should we close this? |
Description
This pull request introduces a new feature to Fiber v2 that enables serving static files directly from an embedded filesystem (embed.FS). The enhancement allows developers to bundle static resources with their Go binary using Go's embed functionality, eliminating the need for external file dependencies in production.
The main change adds a new StaticFilesystem method that accepts an embed.FS instance, making it seamless to serve embedded static files while maintaining consistency with Fiber's existing static file serving API.
Added StaticFilesystem method to support serving static resources from embed.FS
Implemented registerStaticFS helper function in router.go
Maintained backward compatibility with existing static file serving functionality
Fixes # (issue)
Changes introduced
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
Type of change
Please delete options that are not relevant.
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md