chore(deps): bump react-ace from 14.0.1 to 15.0.0 in /superset-frontend - #42833
chore(deps): bump react-ace from 14.0.1 to 15.0.0 in /superset-frontend#42833dependabot[bot] wants to merge 2 commits into
Conversation
dosubot
Bot
added
dependencies:npm
javascript
|
Bito Automatic Review Skipped - Files Excluded |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42833 +/- ##
==========================================
- Coverage 66.38% 65.58% -0.81%
==========================================
Files 2857 2857
Lines 161133 161133
Branches 37064 37064
==========================================
- Hits 106967 105674 -1293
- Misses 52147 53448 +1301
+ Partials 2019 2011 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@dependabot recreate |
a72de3a to
f26f076
Compare
Bumps [react-ace](https://github.com/securingsincity/react-ace) from 14.0.1 to 15.0.0. - [Release notes](https://github.com/securingsincity/react-ace/releases) - [Changelog](https://github.com/securingsincity/react-ace/blob/main/CHANGELOG.md) - [Commits](securingsincity/react-ace@v14.0.1...v15.0.0) --- updated-dependencies: - dependency-name: react-ace dependency-version: 15.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
f26f076 to
b7a7b35
Compare
…fing react-ace 15.0.0 replaces its internal componentDidUpdate deep-equality check (lodash.isequal -> fast-equals's deepEqual) for deciding whether to re-apply the annotations/markers/setOptions/scrollMargin props. Verified via the published package that it is still deepEqual (not shallowEqual), so the semantic intent is preserved -- but there was no test pinning this behavior for the one prop Superset actually relies on it for: AceEditorProvider, EditorWrapper, and several modals (DatabaseModal, AnnotationModal, ThemeModal, PropertiesModal) all pass a freshly mapped annotations array on every render. Adds a regression test asserting both directions: a same-content, different-reference annotations array must not re-trigger session.setAnnotations() (or the editor would thrash on every keystroke-driven re-render), while genuinely different content must still update it. Verified this actually catches a regression by temporarily breaking each direction in the installed react-ace 15.0.0 package and confirming the test fails accordingly, then restoring it. markers itself is never used anywhere in the Superset codebase (only annotations is), so it does not need its own coverage here.
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
Risk assessment: react-ace 14.0.1 → 15.0.0
Went through react-ace's actual published source for both versions (not just the release notes) and every place it's used in this codebase. Summary: low risk, and I've added a regression test for the one part of the bump that genuinely touches Superset's behavior.
The headline breaking change doesn't apply here
v15's release notes call out one breaking change: deep imports (react-ace/lib/...) no longer resolve, since the new ESM exports map only exposes the package root. Checked every react-ace import in the codebase (AsyncAceEditor/index.tsx, types.ts) — Superset only ever does a dynamic import('react-ace') and consumes the default export (AceEditor) plus the IAceEditorProps type. No deep imports, no split/diff named exports. Unaffected.
The change that does matter: internal equality-library swap
react-ace's componentDidUpdate decides whether to re-apply annotations, markers, setOptions, and scrollMargin props via a deep-equality check. 14.0.1 uses lodash.isequal; 15.0.0 replaces it with fast-equals's deepEqual (confirmed by downloading and diffing the actual published lib/ace.js/ace.mjs between versions — the surrounding lifecycle logic is otherwise an unchanged, faithful port). Critically it's deepEqual, not shallowEqual — the semantic intent is preserved.
This matters because Superset's own usage pattern is exactly the case this check exists for: AceEditorProvider.tsx, EditorWrapper, and several modals (DatabaseModal, AnnotationModal, ThemeModal, PropertiesModal's AdvancedSection) all pass a freshly .map()-derived annotations array on every render — same content, new array reference, every time. If the equality semantics ever diverged (e.g. a future swap to shallowEqual, or an edge case in how fast-equals handles this specific {row, column, text, type} shape), the failure mode is either constant unnecessary re-application (perf) or silently stale annotations (a real, easy-to-miss correctness bug — SQL validation errors or JSON schema errors just not updating in the editor).
There was no existing test covering this path at all, despite annotations being one of the most heavily-used props in the whole AsyncAceEditor surface. markers itself is never used anywhere in the codebase, so it doesn't need coverage.
What I added
A new test in AsyncAceEditor.test.tsx that renders SQLEditor, spies on the underlying Ace session's setAnnotations, and asserts both directions:
- a same-content, different-reference
annotationsarray does not re-triggersetAnnotations(guards the perf case) - genuinely different content does trigger it, with the new value (guards correctness)
I verified this is a real regression guard, not a tautology: I installed the actual bumped react-ace 15.0.0 locally, ran the new test (passes), then temporarily patched the installed package to break each direction independently (always-skip and always-apply) and confirmed the test fails accordingly in both cases before restoring the real package. Also ran the full existing AsyncAceEditor suite (24/24 pass) plus the three other test files that reference react-ace indirectly through Superset's wrapper (SqlEditor.test.tsx, CssTemplateModal.test.tsx, DndFilterSelect.test.tsx — 27/27 pass) against the real 15.0.0 install.
Other notes, non-blocking
- TypeScript's
IAceEditorPropsinterface is unchanged field-for-field between versions (diffed the actual.d.tsoutput) — no type-level breakage. - v15's release notes mention ace-builds now loads eagerly on
react-aceimport rather than lazily on first render. Doesn't matter for Superset:AsyncAceEditor's ownimport('react-ace')is already deferred behindAsyncEsmComponent, so this is already lazy at the point where it matters. - v14.1.0 (included in this bump's range) fixed a real upstream bug: null/absent
markershandling. Moot for Superset sincemarkersisn't used, but worth knowing it's a net-positive line item in the bump, not a risk.
No blockers found.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #45e92fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
2 participants
Bumps react-ace from 14.0.1 to 15.0.0.
Release notes
Sourced from react-ace's releases.
Commits
334f49eMerge pull request #2125 from securingsincity/esm-supporta60deb2Add ESM Supportf475e45Update npm-publish9617b4aMerge pull request #2124 from securingsincity/dependency-rethinke89fd5fFix test301584fHarden GitHub Actions workflows78c8adfUpgrade actions012d1e1Update CI workflows for new build pipeline37e7bf8Upgrade dependencies and build pipelineeab885bMerge pull request #2118 from itkrt2y/mainMaintainer changes
This version was pushed to npm by GitHub Actions, a new releaser for react-ace since your current version.
Install script changes
This version modifies
preparescript that runs during installation. Review the package contents before updating.