Skip to content

[ZEPPELIN-6735] Fix code editor model leaks and diff updates - #5574

Open
miinhho wants to merge 1 commit into
apache:masterfrom
miinhho:ZEPPELIN-6735-code-editor-model-lifecycle
Open

miinhho wants to merge 1 commit into
apache:masterfrom
miinhho:ZEPPELIN-6735-code-editor-model-lifecycle

Conversation

@miinhho

@miinhho miinhho commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What is this PR for?

The shared code editor leaves its Monaco text models behind when destroyed. This affects the regular paragraph editor.

In diff mode, the initial original and modified text are reversed. Value changes create new models, and changing only nzOriginalText does not update the original text.

This PR disposes of component-owned models, fixes the diff orientation, and updates the existing models when either input changes.

What type of PR is it?

Bug Fix

Todos

  • Fix model disposal and diff initialization and updates.
  • Add regression tests.

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-6735

How should this be tested?

Run the regression tests from zeppelin-web-angular:

npm run test:shell -- src/app/share/code-editor/code-editor.component.spec.ts

Make sure that the tests pass for these behaviors:

  • Diff mode displays the original text on the left and the modified text on the right.
  • Value changes update the existing models rather than create new ones.
  • Changing only nzOriginalText leaves the modified text unchanged.
  • Destroying the component disposes of its models but leaves caller-owned models intact.
  • Normal-mode writes update the attached model after a caller replaces it.

Questions:

  • Does the license files need to update? No.
  • Is there breaking changes for older versions? No.
  • Does this needs documentation? No.

@voidmatcha voidmatcha 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.

LGTM 👍

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants