Repository navigation
feat: allow disabling automatic config discovery #593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| /** | ||
| * Copyright (c) 2026 Google LLC | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy of | ||
| * this software and associated documentation files (the "Software"), to deal in | ||
| * the Software without restriction, including without limitation the rights to | ||
| * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of | ||
| * the Software, and to permit persons to whom the Software is furnished to do so, | ||
| * subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included in all | ||
| * copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS | ||
| * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR | ||
| * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER | ||
| * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN | ||
| * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
| */ | ||
|
|
||
| import * as fs from "node:fs/promises"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import { expect } from "chai"; | ||
| import connect from "connect"; | ||
| import request from "supertest"; | ||
|
|
||
| import superstatic from "../../src/"; | ||
|
|
||
| describe("explicit middleware configuration", () => { | ||
| let originalCwd: string; | ||
| let directory: string; | ||
|
|
||
| beforeEach(async () => { | ||
| originalCwd = process.cwd(); | ||
| directory = await fs.mkdtemp(join(tmpdir(), "superstatic-config-")); | ||
| await fs.mkdir(join(directory, "public")); | ||
| await fs.writeFile( | ||
| join(directory, "public", "index.html"), | ||
| "explicit config", | ||
| ); | ||
| process.chdir(directory); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| process.chdir(originalCwd); | ||
| await fs.rm(directory, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| for (const filename of ["superstatic.json", "firebase.json"]) { | ||
| it(`does not inherit headers or rewrites from ${filename} with autoConfig disabled`, async () => { | ||
| const fileConfig = { | ||
| headers: [ | ||
| { source: "**", headers: [{ key: "X-Autoloaded", value: "yes" }] }, | ||
| ], | ||
| rewrites: [{ source: "**", destination: "/index.html" }], | ||
| }; | ||
| await fs.writeFile( | ||
| filename, | ||
| JSON.stringify( | ||
| filename === "firebase.json" ? { hosting: fileConfig } : fileConfig, | ||
| ), | ||
| ); | ||
| const options = { | ||
| autoConfig: false, | ||
| fallthrough: false, | ||
| cwd: directory, | ||
| config: { | ||
| public: "public", | ||
| redirects: [{ source: "/to-index", destination: "/index.html" }], | ||
| }, | ||
| }; | ||
| const app = connect().use(superstatic(options)); | ||
|
|
||
| const response = await request(app) | ||
| .get("/index.html") | ||
| .expect(200) | ||
| .expect("explicit config"); | ||
| expect(response.headers).not.to.have.property("x-autoloaded"); | ||
| await request(app) | ||
| .get("/to-index") | ||
| .expect(301) | ||
| .expect("Location", "/index.html"); | ||
| await request(app).get("/missing").expect(404); | ||
| }); | ||
| } | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,4 +120,91 @@ describe("loading config files", () => { | |
| await fs.rm("firebase.json"); | ||
| }); | ||
| }); | ||
| describe("automatic config discovery", () => { | ||
| let originalCwd; | ||
|
|
||
| beforeEach(() => { | ||
| originalCwd = process.cwd(); | ||
| process.chdir(".tmp"); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.chdir(originalCwd); | ||
| }); | ||
|
|
||
| for (const filename of ["superstatic.json", "firebase.json"]) { | ||
| it(`does not merge ${filename} into an explicit object`, async () => { | ||
| const fileConfig = { public: "default", headers: [{ source: "**" }] }; | ||
| await fs.writeFile( | ||
| filename, | ||
| JSON.stringify( | ||
| filename === "firebase.json" ? { hosting: fileConfig } : fileConfig, | ||
| ), | ||
| ); | ||
|
|
||
| expect(loadConfigFile({ public: "app" }, false)).to.eql({ | ||
| public: "app", | ||
| }); | ||
| expect(loadConfigFile(JSON.stringify({ public: "app" }), false)).to.eql( | ||
| { | ||
| public: "app", | ||
| }, | ||
| ); | ||
| expect(loadConfigFile({}, false)).to.eql({}); | ||
| expect(loadConfigFile(undefined, false)).to.eql({}); | ||
| }); | ||
|
|
||
| for (const autoConfig of [undefined, true]) { | ||
| it(`still merges ${filename} when autoConfig is ${autoConfig}`, async () => { | ||
| const fileConfig = { public: "default", cleanUrls: true }; | ||
| await fs.writeFile( | ||
| filename, | ||
| JSON.stringify( | ||
| filename === "firebase.json" | ||
| ? { hosting: fileConfig } | ||
| : fileConfig, | ||
| ), | ||
| ); | ||
|
|
||
| expect(loadConfigFile({ public: "app" }, autoConfig)).to.eql({ | ||
| public: "app", | ||
| cleanUrls: true, | ||
| }); | ||
| }); | ||
|
|
||
| it(`merges a stringified object with ${filename} when autoConfig is ${autoConfig}`, async () => { | ||
| const fileConfig = { public: "default", cleanUrls: true }; | ||
| await fs.writeFile( | ||
| filename, | ||
| JSON.stringify( | ||
| filename === "firebase.json" | ||
| ? { hosting: fileConfig } | ||
| : fileConfig, | ||
| ), | ||
| ); | ||
|
|
||
| expect( | ||
| loadConfigFile(JSON.stringify({ public: "app" }), autoConfig), | ||
| ).to.eql({ | ||
| public: "app", | ||
| cleanUrls: true, | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| it("still loads an explicit config filename", async () => { | ||
| await fs.writeFile( | ||
| "custom.json", | ||
| JSON.stringify({ hosting: { public: "app" } }), | ||
| ); | ||
| expect(loadConfigFile("custom.json", false)).to.eql({ public: "app" }); | ||
| }); | ||
|
|
||
| it("still loads a stringified config object", () => { | ||
| expect(loadConfigFile(JSON.stringify({ public: "app" }), false)).to.eql({ | ||
| public: "app", | ||
| }); | ||
| }); | ||
|
Comment on lines
+204
to
+208
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To ensure that stringified JSON config objects are correctly merged with the default config files when it("still loads a stringified config object", () => {
expect(loadConfigFile(JSON.stringify({ public: "app" }), false)).to.eql({
public: "app",
});
});
it("merges a stringified config object with default config files when autoConfig is true", async () => {
await fs.writeFile(
"superstatic.json",
JSON.stringify({ cleanUrls: true }),
);
try {
expect(loadConfigFile(JSON.stringify({ public: "app" }), true)).to.eql({
public: "app",
cleanUrls: true,
});
} finally {
await fs.rm(
"superstatic.json",
{ force: true },
);
}
});
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added four failing-before/passing-after cases covering both default filenames with autoConfig omitted or explicitly true, plus coverage for JSON strings with discovery disabled. All 18 loader tests pass, and the full Linux suite passes with 247 tests and 15 pending. Build and lint also pass. |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bug in how stringified JSON config objects (e.g.,
'{\"public\": \"app\"}') are handled.Currently, if a stringified JSON object is passed,
JSON.parse(filename)succeeds, soconfigObjectis populated. However,filenameremains the original JSON string. Because it is a string, it bypasses thecatchblock, and later fails theisPlainObject(filename)check on line 58. As a result, the default config files are never loaded or merged, which contradicts the intended behavior described in the comments.Furthermore, even if
isPlainObject(filename)on line 58 were to evaluate totrue, settingfilename = defaultFiles(an array) at that point is too late because theArray.isArray(filename)resolution block has already executed, meaning the array would never be resolved to an actual file path.To fix both issues, we should set
filename = defaultFilesimmediately when a stringified JSON object is successfully parsed in thetryblock.Here is the recommended refactoring for the parsing block:
This ensures that both plain objects and stringified JSON objects are correctly identified and set to
defaultFilesbefore the array resolution logic runs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I normalized parsed JSON objects before filename resolution in 5edcfd0. They now use the same default-file merge path as object configs, while autoConfig: false still bypasses discovery. Supplied values override defaults in both forms.