Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editor: Fixing bug that caused infinite loop during preview #145

Merged
merged 4 commits into from
Dec 1, 2024

Conversation

berhalak
Copy link
Contributor

@berhalak berhalak commented Nov 20, 2024

When an inner widget was reporting some mandatory columns to map, the builder was causing infinite loop. More in comment, but here is summary.

  • Builder was sending ready message to Grist, without any mapped columns
  • Grist was rendering the widget, sending JS/HTML back
  • Builder was creating inner widget, which called ready method and reported some mandatory columns to map
  • Grist unloads builder, as columns where not mapped
  • User mapped a column
  • Grist loads builder
  • Builder reports back to Grist that no columns to map (and no mandatory columns anymore).
  • Grist unloads builder once again (as mandatory option has changed, and Grist is sensitive here).
  • Grist sees no mandatory columns, so shows builder once again.
  • Builder, reports back no mandatory columns, creates inner widget, this inner widget reports mandatory columns
  • And it loops

Here the basic idea was not to call the configure endpoint from the builder, instead builder is injecting its configuration in the configure call from the inner widget, that way Grist sees only one configure call, and columns mapping stays consistent during reloads.

Not related:

  • Some dead code is removed
  • api_deps.js is loaded with monaco
  • moncaco scripts are loaded in parallel

Here is a document with this builder and markdown widget's content inside (with mandatory column):
https://public.getgrist.com/d1t1x4BvxuMK/JsFiddle

Verified

This commit was signed with the committer’s verified signature.
KrishVora01 Krish Vora
Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for boisterous-sunburst-a5c941 ready!

Name Link
🔨 Latest commit 4388426
🔍 Latest deploy log https://app.netlify.com/sites/boisterous-sunburst-a5c941/deploys/6746e208a94d520008176f9d
😎 Deploy Preview https://deploy-preview-145--boisterous-sunburst-a5c941.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.

Verified

This commit was signed with the committer’s verified signature.
KrishVora01 Krish Vora
@georgegevoian georgegevoian self-requested a review November 22, 2024 22:23
berhalak and others added 2 commits November 27, 2024 10:10

Verified

This commit was signed with the committer’s verified signature.
KrishVora01 Krish Vora
Co-authored-by: George Gevoian <[email protected]>

Verified

This commit was signed with the committer’s verified signature.
KrishVora01 Krish Vora
Co-authored-by: George Gevoian <[email protected]>
@georgegevoian georgegevoian merged commit 983ae45 into master Dec 1, 2024
1 of 2 checks passed
@georgegevoian georgegevoian deleted the fixing-loop-mapped-columns branch December 1, 2024 23:06
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.

None yet

2 participants