chore: remove references to node.parent #14395
Conversation
|
|
preview: https://svelte-dev-git-preview-svelte-14395-svelte.vercel.app/ this is an automated message |
|
| node.metadata.path = [...context.path]; | ||
|
|
||
| context.state.analysis.elements.push(node); | ||
| context.state.analysis.elements.push({ node, path: context.path }); |
There was a problem hiding this comment.
We already have path as metadata on the node, would be better to either reuse that or to replace usages of metadata.path where possible.
(having metadata.path might make it easier for the CSS prune stuff)
There was a problem hiding this comment.
As far as I can see we're using metadata.path in only one place. Seems like we could probably get rid of it. Using metadata.path in pruning is a possibility but it seems easier to just use context.path everywhere instead
There was a problem hiding this comment.
Actually I take that back, I misunderstood — reverting most recent change
There was a problem hiding this comment.
but it seems easier to just use context.path everywhere instead
But during pruning context.path will relate to the css tree and not the element's parent tree
There was a problem hiding this comment.
I meant the context.path being stored on analysis.elements. But on reflection it does make sense to use metadata.path everywhere — you probably missed that reply because GitHub's UI is a piece of shit
This reverts commit c0c7ab8.
… bewildering code
|
going to try and tackle #14399 as part of this PR. there's a lot of pretty weird code in |
… HTML (#14468) fixes #14466 The logic introduced in #14395 was flawed - not every text or expression outside the template is the child of an attribute. This turns it around: We know that every child of a fragment is inside the template, so we ignore all text/expression tags that are not child of a fragment
Per #14276 and #14271, we currently have a bug caused by
node.parentbeing incorrect during CSS pruning.One idea is to allow the AST to be mutated rather than replaced, so that links between nodes creating before traversal happens can be preserved. While it might fix this specific issue, I'm confident that it will lead to more problems down the road.
Instead, I propose simply getting rid of
node.parentandnode.prev. They're a hacky solution to the problem that adds overhead and complicates the types, and we still wouldn't have guarantees that the links would remain valid after traversal. Instead, we can usecontext.pathin the places where we would usenode.parent.This PR doesn't touch the CSS pruning code, which is where the bulk of
node.parent(and all ofnode.prev) usage happens — untangling that stuff is a little bit hairier. I had a WIP branch locally that did it, and it became stale, so I figured I'd try doing the simple bit first and then coming back to the CSS stuff rather than trying again to do it all in one go.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint