Skip to content

fix(format): preserve literal replacement characters in values - #21751

Merged
plainheart merged 2 commits into
apache:releasefrom
Hanabi9248:codex/literal-template-values
Sep 12, 2026
Merged

plainheart merged 2 commits into
apache:releasefrom
Hanabi9248:codex/literal-template-values

Conversation

@Hanabi9248

Copy link
Copy Markdown

Brief Information

  • bug fixing
  • new feature
  • others

What does this PR do?

Preserves literal dollar sequences in values inserted into formatter templates.

Details

Before: What was the problem?

Formatting {a} with a series name of Price $$ returns Price $. A name containing $& can reinsert the placeholder instead of displaying the name, including when HTML encoding is enabled.

Both formatTpl and formatTplSimple pass values directly as replacement strings, so String.replace interprets these sequences.

After: How does it behave after the fixing?

Callback replacements insert the values literally. HTML encoding and indexed series formatting stay unchanged.

Document Info

  • This PR doesn't relate to document changes

Misc

Related test cases or examples to use the new APIs

Three tests cover literal replacement sequences with and without HTML encoding, simple templates, indexed series, and numeric values. All three fail before the fix.

Validation: 197 unit tests pass; npm run build:lib, npm run checktype, ESLint on the changed files, and git diff --check pass. Browser visual tests were not run.

Merging options

  • Please squash the commits into a single one when merging.

@echarts-bot

echarts-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

Please DO NOT commit the files in dist, i18n, and ssr/client/dist folders in a non-release pull request. These folders are for release use only.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused implementation correctly resolves the replacement-string issue with adequate regression coverage.

Pull request overview

Fixes formatter substitutions so dollar sequences in values remain literal.

Changes:

  • Uses callback replacements in both template formatters.
  • Adds coverage for HTML encoding, indexed series, and numeric values.
File summaries
File Description
src/util/format.ts Preserves literal replacement characters.
test/ut/spec/util/format.test.ts Adds formatter regression tests.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@plainheart plainheart left a comment

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.

Hi, thanks for your contribution! I plan to include it in v6.1.1. Can you change the target branch to the release branch?

@plainheart plainheart added this to the 6.1.1 milestone Sep 12, 2026
Signed-off-by: Hanabi <317387557+Hanabi9248@users.noreply.github.com>
Signed-off-by: Hanabi <317387557+Hanabi9248@users.noreply.github.com>
@github-actions
github-actions Bot force-pushed the codex/literal-template-values branch from a36061b to 5e78e20 Compare September 12, 2026 02:40
@Hanabi9248
Hanabi9248 changed the base branch from master to release September 12, 2026 02:41
@Hanabi9248

Copy link
Copy Markdown
Author

Retargeted to release and rebased the two commits onto it. The diff still contains only the formatter fix and its tests. All 197 unit tests, the library build, type checks, and scoped ESLint pass on the release base. Thanks!

@plainheart
plainheart merged commit ddfd8f8 into apache:release Sep 12, 2026
2 checks passed
@echarts-bot

echarts-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants