fix(model): don't require TimelineComponent for media-only option. close #21686 - #21749
Open
MohammadWasi wants to merge 1 commit into
Open
fix(model): don't require TimelineComponent for media-only option. close #21686#21749MohammadWasi wants to merge 1 commit into
MohammadWasi wants to merge 1 commit into
Conversation
apache#21686 When an option uses the full `{ baseOption, media }` form without a `timeline`, `OptionManager.parseRawOption` injects an empty `timeline` key into `baseOption` (`baseOption.timeline = timelineOnRoot`, where `timelineOnRoot` is `undefined`). `checkMissingComponents` then iterated that key and wrongly reported `TimelineComponent` as "used but not imported", even though no timeline is configured. Guard `checkMissingComponents` so component options whose value is `null`/`undefined` are not treated as used. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for your contribution! 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. |
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.
Brief Information
This pull request is in the type of:
What does this PR do?
Stops ECharts from wrongly requiring
TimelineComponentwhen an option uses the{ baseOption, media }form without atimeline.Fixed issues
baseOptionwithmediaincorrectly requiresTimelineComponentwhen no timeline is configuredDetails
Before: What was the problem?
Using the modular build without registering
TimelineComponent, an option structured withbaseOption+mediabut notimelinereported:Depending on the environment this surfaces as an exception during
setOption, forcing users to register a component they don't use.Root cause: in
OptionManager.parseRawOption, when abaseOptionis declared, the parser injects the root timeline into it for merge purposes:This leaves an own
timelinekey onbaseOptionwhose value isundefined.checkMissingComponentsthen iterates every key of the option and reports anymainTypewhose class isn't registered — so the emptytimelineentry is wrongly flagged as a missingTimelineComponent. It only reproduces with the{ baseOption, media }form because the plain root-option path doesn't inject that key.After: How does it behave after the fixing?
checkMissingComponentsnow skips component options whose value isnull/undefined, since such an entry means the component isn't actually used. An option withbaseOption+mediaand notimelineworks with the modular build without registeringTimelineComponent. Genuine missing-component reporting is unchanged (a realtimeline: {...}still reports when unregistered).The fix is guarded by
__DEV__(this check only runs in development builds), so there is no production behavior change.Regression test
Added a case to
test/ut/spec/model/componentMissing.test.tsasserting that a{ baseOption, media }option without a timeline does not reportTimelineComponentas missing. Verified it fails onmaster(reproduces the bug) and passes with this fix. The fulltest/ut/spec/modelsuite passes, along withnpm run lintandnpm run checktype.Document Info
One of the following should be checked.
Misc
Security Checking
ZRender Changes
Related test cases or examples to use the new APIs
Added a Jest regression test in
test/ut/spec/model/componentMissing.test.ts.