Skip to content

Commit

Permalink
fix(config): pass optional templates through during partial resolution
Browse files Browse the repository at this point in the history
Previously, we would immediately resolve an optional template string
as undefined during partial resolution, which would cause problems when
using optional template strings for variables that are only available
later in the process. We now leave them unchanged until the final
resolution occurs.

Fixes #2241
  • Loading branch information
edvald authored and thsig committed Feb 4, 2021
1 parent 5a21894 commit c4dac8b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
4 changes: 2 additions & 2 deletions core/src/template-string-parser.pegjs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ FormatString
// We allow certain configuration contexts (e.g. placeholders for runtime.*) to indicate that a template
// string should be returned partially resolved even if allowPartial=false.
return text()
} else if (options.allowPartial) {
return text()
} else if (allowUndefined) {
if (e && e._error) {
return { ...e, _error: undefined }
} else {
return e
}
} else if (options.allowPartial) {
return text()
} else if (e && e._error) {
return e
} else {
Expand Down
27 changes: 27 additions & 0 deletions core/test/unit/src/template-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,21 @@ describe("resolveTemplateString", async () => {
expect(res).to.equal("value")
})

it("should correctly resolve if ? suffix is present but value exists", async () => {
const res = resolveTemplateString("${foo}?", new TestContext({ foo: "bar" }))
expect(res).to.equal("bar")
})

it("should allow undefined values if ? suffix is present", async () => {
const res = resolveTemplateString("${foo}?", new TestContext({}))
expect(res).to.equal(undefined)
})

it("should pass optional string through if allowPartial=true", async () => {
const res = resolveTemplateString("${foo}?", new TestContext({}), { allowPartial: true })
expect(res).to.equal("${foo}?")
})

it("should interpolate a format string with a prefix", async () => {
const res = resolveTemplateString("prefix-${some}", new TestContext({ some: "value" }))
expect(res).to.equal("prefix-value")
Expand Down Expand Up @@ -1020,6 +1030,23 @@ describe("resolveTemplateStrings", () => {
})
})

it("should correctly handle optional template strings", async () => {
const obj = {
some: "${key}?",
other: "${missing}?",
}
const templateContext = new TestContext({
key: "value",
})

const result = resolveTemplateStrings(obj, templateContext)

expect(result).to.eql({
some: "value",
other: undefined,
})
})

it("should collapse $merge keys on objects", () => {
const obj = {
$merge: { a: "a", b: "b" },
Expand Down

0 comments on commit c4dac8b

Please sign in to comment.