Skip to content

Conversation

@LukasBoll
Copy link
Contributor

This commit addresses an issue where array translations were created as new objects within the core module for each render cycle, causing unnecessary rerenders. By memoizing the translation object in the material renderer set, this commit optimizes performance and prevents redundant rerenders.

@netlify
Copy link

netlify bot commented Jul 23, 2024

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 0e697e7
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/66be575ee1c5a000082d87f5
😎 Deploy Preview https://deploy-preview-2358--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sdirix
Copy link
Member

sdirix commented Aug 14, 2024

@LukasBoll Can you fix the build or do you need support?

@LukasBoll LukasBoll force-pushed the resolve_translation_rerender branch 4 times, most recently from 862b6bc to 0e697e7 Compare August 15, 2024 19:30
@coveralls
Copy link

coveralls commented Aug 15, 2024

Coverage Status

coverage: 81.733% (-1.6%) from 83.285%
when pulling c64196b on LukasBoll:resolve_translation_rerender
into 52b843a on eclipsesource:master.

@LukasBoll LukasBoll force-pushed the resolve_translation_rerender branch from 0e697e7 to 6f275fb Compare August 15, 2024 20:38
@netlify
Copy link

netlify bot commented Aug 15, 2024

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit c64196b
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/66e4636c30eae20008c99350
😎 Deploy Preview https://deploy-preview-2358--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@LukasBoll
Copy link
Contributor Author

Hi @sdirix,
I just fixed the build issues :)

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

The PR looks good in general. However there seems to be an issue in Vue Vanilla. When checking the "Nested arrays" example and adding a new element to the outer array, then the nested array within it crashes. Can you check what's going on?

This is the error message:

TypeError: Cannot read properties of undefined (reading 'noDataMessage')
at Proxy.render$l (ArrayListRenderer.vue:44:31)
at renderComponentRoot (runtime-core.esm-bundler.js:887:16)
at ReactiveEffect.componentUpdateFn [as fn] (runtime-core.esm-bundler.js:6020:46)
at ReactiveEffect.run (reactivity.esm-bundler.js:177:19)
at instance.update (runtime-core.esm-bundler.js:6151:16)
at setupRenderEffect (runtime-core.esm-bundler.js:6161:5)
at mountComponent (runtime-core.esm-bundler.js:5929:7)
at processComponent (runtime-core.esm-bundler.js:5883:9)
at patch (runtime-core.esm-bundler.js:5351:11)
at ReactiveEffect.componentUpdateFn [as fn] (runtime-core.esm-bundler.js:6027:11)

This commit addresses an issue where array translations were created as new objects within the core module for each render
cycle, causing unnecessary rerenders. By memoizing the translation object in the material renderer set, this commit prevents redundant rerenders.
This commit also adds memorization to the attributes of the material oneOfRenderer to further avoid unnecessary rerendering.

closes eclipsesource#2342
@LukasBoll LukasBoll force-pushed the resolve_translation_rerender branch from 6f275fb to 0523417 Compare August 18, 2024 21:00
@LukasBoll LukasBoll requested a review from sdirix August 19, 2024 06:41
@sdirix
Copy link
Member

sdirix commented Aug 21, 2024

@LukasBoll Can you resolve the conflicts?

@sdirix sdirix self-assigned this Sep 11, 2024
@sdirix sdirix added this to the 3.4 milestone Sep 11, 2024
; Conflicts:
;	packages/vanilla-renderers/src/complex/array/ArrayControlRenderer.tsx
@lucas-koehler
Copy link
Contributor

Merge commit only fixes a tiny merge conflict.

@lucas-koehler lucas-koehler merged commit 8cdd5ef into eclipsesource:master Sep 13, 2024
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.

4 participants