Repository navigation
Editors: get rid of the ctor overrides - #34925
Merged
EugeniyKiyashko merged 6 commits intoAug 26, 2026
Merged
EugeniyKiyashko merged 6 commits into
EugeniyKiyashko merged 6 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the internal Editor initialization to rely on the framework’s _createElement lifecycle hook instead of overriding ctor, aligning Editor with the DOMComponent construction flow where _createElement is invoked before options initialization. It also adds Jest coverage to ensure validation-related fields are initialized at the correct time and behave as expected.
Changes:
- Removed
Editor.ctor(...)override and movedvalidationRequest/showValidationMessageTimeoutinitialization into_createElement(...). - Ensured the element is still marked as a validation target during element creation.
- Added Jest tests to validate initialization order and
validationRequestfiring behavior on value changes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/ui/editor/editor.ts | Moves per-instance initialization from ctor to _createElement so it runs during DOMComponent element creation (before options init). |
| packages/devextreme/js/__internal/ui/editor/tests/editor.test.ts | Adds Jest tests covering initialization ordering, validation target marking, and validationRequest behavior on value updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
TextEditorBase.ctor validated the buttons option, built the button collection and reset the button/label containers before delegating to DOMComponent.ctor. All of it moves into a new _init() override: _init() still runs inside the same construction call and before _initMarkup(), so _renderButtonContainers() finds the collection, and every descendant override (text_editor.mask, drop_down_editor, select_box) calls super._init() first, so the base keeps running earlier than subclass init. The buttons option is now read through this.option() destructuring instead of the raw options bag, which makes the `if (options)` guard unnecessary - the option store is initialized by the time _init() runs. Adds jest coverage for the button collection being ready before the markup is rendered, for the declared button surviving a repaint and for the E1053 check, so moving the setup to render time cannot slip through unnoticed. Refactoring card: DevExpress/devextreme-private#3836
TextBox.ctor cached the user-provided showClearButton before delegating to DOMComponent.ctor. Search mode force-enables the clear button, and that cache is what tells "the user asked for it" from "search mode turned it on", so the value has to be captured before the defaults are merged in - reading the merged option later cannot reproduce it. _initOptions() receives the same raw option bag inside the same construction call, so the capture moves there as is. The `if (options)` guard is dropped: by that point the bag is always an object, Component.ctor defaults it. Adds jest coverage for both sides of the distinction - search mode enables the clear button when the option is omitted and leaves it alone when it is passed explicitly (T218573) - so switching to the merged value cannot pass unnoticed. Refactoring card: DevExpress/devextreme-private#3836
Both ctors existed only to raise a deprecation warning after delegating to
DOMComponent.ctor, and both need the raw option bag: they ask whether the user
mentioned the option at all ('preventScrollEvents' in options, 'provider' in
options), which the merged value cannot answer.
_initOptions() receives that same raw bag inside the same construction call, so
the checks move there unchanged. The `if (options)` guards are dropped - the bag
is always an object by then. The warning is now raised while options are being
processed instead of after the first render; for a log line that is immaterial,
and the existing coverage pins it: the overlay preventScrollEvents module
(including the _ignorePreventScrollEventsDeprecation cases) and the dxMap
"provider is set to bing on init/runtime" tests.
With this, js/__internal/ui no longer declares ctor anywhere.
Refactoring card: DevExpress/devextreme-private#3836
EugeniyKiyashko
changed the title
Raushen
reviewed
Aug 26, 2026
Review follow-up: creating the validationRequest callbacks while the element is being created reads as a side effect. _init() is the canonical place and it is still early enough - DOMComponent.endUpdate() calls super.endUpdate() (which reaches _initializeComponent -> _init) before _updateDOMComponent() renders. Nothing observes the two fields in the window this opens up: the only place that fires validationRequest is _optionChanged, which _notifyOptionChanged gates behind _initialized - set after _init() returns; initValidationOptions() is a pure options transform; and the external subscriber is added by DefaultAdapter once the editor exists. _init() also cannot run twice, _initialized is only ever set to true, so the callbacks object cannot be replaced under a subscriber. The jest case is repinned from "before the options are initialized" - that was the status quo, not a requirement - to "before the markup is rendered".
Reverts the move to _init() from the previous commit - it broke the DxValidator "should work with dx-validator" angular tests. devextreme-angular passes an onInitializing handler that calls beginUpdate() on the widget itself (packages/devextreme-angular/src/core/component.ts), so the editor's _init() is deferred until the wrapper calls endUpdate() in its own ngAfterViewInit. A nested <dx-validator> is a child component and Angular runs child ngAfterViewInit first, so the validator binds to the editor while its _init() still has not run: DefaultAdapter then calls .add on an undefined validationRequest. _createElement() is inside the constructor proper and is not affected by the update lock, which is why the callbacks belong there - next to the registration that makes the element a validation target in the first place. The ordering probe is replaced with a test that reproduces the same flow without Angular: the editor is created with an onInitializing that calls beginUpdate(), a Validator is attached while initialization is still deferred, and the editor is released with endUpdate(). It fails with the same TypeError if the setup is moved to _init().
Raushen
approved these changes
Aug 26, 2026
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.
No description provided.