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

fix weird Stack rendering issue #2868

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Conversation

theoephraim
Copy link
Contributor

very odd issue only visible when running the built version of the app where some children of Stack were not being re-rendered. I didn't manage to get a minimal reproduction put together, but I was eventually able to get it fixed 🤷

@github-actions github-actions bot added the A-web label Oct 16, 2023
@@ -13,6 +13,7 @@
<br />
workspace settings (gear in top right) > "Import Workspace"
</p>
<VButton icon="check" @click="close">Close this window</VButton>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

undoing zack's changes that were just a workaround for the issue

async function continueHandler() {
await moduleStore.EXPORT_WORKSPACE();
function continueHandler() {
moduleStore.EXPORT_WORKSPACE();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will re-enable the no floating promises eslint rule soon, but unrelated to this, so leaving it as is to match other similar uses of api actions.


// NOTE - ran into a weird errors that only appeared on the built version of the app
// but resolved it by adding this Fragment wrapper around each child
wrappedChildren.push(h(Fragment, {}, [children[i]]));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this Fragment wrapper is the magic that fixed it... Got the idea from vuejs/core#8298 but never was fully able to wrap my head around what exactly is triggering the issue.

const children = [] as VNode[];
rawChildren.forEach((child) => {
// components that are hidden via v-if end up as a Comment <!--v-if--> so we skip over them
if (child.type === Comment) return;

// if the child is a template with a v-for, we actually want to get all of its children
if (child.type === Fragment) {
children.push(...(child.children as VNode[]));
children.push(...unwrapChildren(child.children as VNode[]));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this recursive unwrapping is something I realized was missing to handle nested loops and templates

@theoephraim
Copy link
Contributor Author

bors merge

@si-bors-ng
Copy link
Contributor

si-bors-ng bot commented Oct 17, 2023

Build succeeded:

@si-bors-ng si-bors-ng bot merged commit aa41864 into main Oct 17, 2023
8 checks passed
@si-bors-ng si-bors-ng bot deleted the theo/stack-rendering-weirdness branch October 17, 2023 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant