Skip to content

fix linkedom + inline usage #1

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: feat/linkedom
Choose a base branch
from
Open

fix linkedom + inline usage #1

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
64e43a3
use linkedom with patch
Jun 11, 2024
470d215
a
Jun 12, 2024
49cb24d
fix plugins/inline's use of linkedom
Jun 12, 2024
1c382d9
revert deps
Jun 12, 2024
1d48f2c
Update snapshots because they look similar
Jun 18, 2024
132d998
add workaround for linkedom and standard DOM API
Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deps/dom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/// <reference lib="dom" />
/// <reference lib="dom.iterable" />
// export * from "https://deno.land/x/[email protected]/deno-dom-wasm.ts";
export * from "https://esm.sh/[email protected]";
export * from "https://esm.sh/[email protected]";
2 changes: 1 addition & 1 deletion plugins/inline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function (userOptions?: Options) {
page.document!.querySelectorAll("template"),
)
.flatMap((template) =>
Array.from(template.content.querySelectorAll(selector))
Array.from(template.querySelectorAll(selector))
Copy link
Author

@iacore iacore Jun 12, 2024

Choose a reason for hiding this comment

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

this usage may or may not be unique to linkedom.

Copy link

@gilesdring gilesdring Jun 19, 2024

Choose a reason for hiding this comment

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

I think it is - and looks like it's broken, according to https://developer.mozilla.org/en-US/docs/Web/API/HTMLTemplateElement.

I wonder if we ought to do something like the following to deal with this scenario...

template.content?.querySelectorAll(selector) || template.querySelectorAll(selector)

Copy link

@gilesdring gilesdring Jun 19, 2024

Choose a reason for hiding this comment

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

Actually, not sure that works...

Copy link

@gilesdring gilesdring Jun 19, 2024

Choose a reason for hiding this comment

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

This is a bit more robust than a previous suggestion...

Suggested change
Array.from(template.querySelectorAll(selector))
{
let inlineElements = Array.from(template.content.querySelectorAll(selector));
// Test for Linkedom
if(inlineElements.map(x => x.parentElement).filter(x => x === null).length > 0) {
inlineElements = Array.from(template.querySelectorAll(selector));
};
return inlineElements;
}

Copy link
Author

@iacore iacore Jun 19, 2024

Choose a reason for hiding this comment

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

I don't think the code is correct actually, since template.content will move the children nodes to a fragment in linkedom.

One workaround is to do

template.childNodes.length === 0 ? template.content.querySelectorAll(_) : template.querySelectorAll(_)

I've talked to @WebReflection about this: WebReflection/linkedom#279

Copy link

@WebReflection WebReflection Jun 19, 2024

Choose a reason for hiding this comment

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

because template.content moves children indeed (once) it's a matter of tradeoffs:

  • are you sure no .content has been accessed previously? use template.qSA
  • do you want to move content ? always reach content
  • do you expect changes to be applied? I still need to understand the use case but this might be on LinkeDOM side of affairs, it's just that I have zero time these days to look at it, I am afraid

Copy link
Author

@iacore iacore Jun 19, 2024

Choose a reason for hiding this comment

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

what is template.qSA?

i can't find it in the source code. we use the TypeScript version directly.

Copy link

@WebReflection WebReflection Jun 19, 2024

Choose a reason for hiding this comment

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

qSA is template.querySelectorAll, sorry for the terseness

);
for (
const element of [
Expand Down
4 changes: 2 additions & 2 deletions tests/__snapshots__/inline.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,10 @@ const a = 1, _b = 1 < a, _c = a > 4;
const a = 1, _b = 1 < a, _c = a > 4;
</script>
<template>
<svg width="126px" height="126px" viewbox="0 0 126 126" version="1.1" xmlns="http://www.w3.org/2000/svg" xlink="http://www.w3.org/1999/xlink" id="el-14" class="has-svg">
<svg class="has-svg" id="el-14" width="126px" height="126px" viewBox="0 0 126 126" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<title>favicon</title>
<g id="favicon" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
<path d="M52.8683497,7.99450751 C48.9069025,24.0969979 69.1669662,21.1536885 69.1669662,44.7198348 C75.794917,37.1051328 70.4463322,26.8453748 81.058456,17.1725913 C81.058456,39.3863344 102.155357,38.1365645 94.0121919,69.2249916 C97.7866664,66.9118477 100.501055,63.103682 102.155357,57.8004944 C105.491398,63.103682 104.245257,72.5297487 98.416934,86.0786945 C103.547554,83.9150474 106.558165,80.9232746 108.378132,76.7271327 C109.04106,105.964869 89.5062156,126.366871 61.9867103,125.995028 C34.467205,125.623185 13.5784967,101.642443 18.2649214,71.802957 C18.9624768,74.9072731 21.5182769,78.5533566 25.1832626,80.0729101 C18.6891247,50.7321037 30.922502,49.6337285 36.0056643,33.9730813 C35.1796585,42.297059 36.0056643,46.924952 40.0208236,51.7593505 C40.7309276,49.6474958 42.0145668,47.6388953 43.3724725,45.627895 L43.7620351,45.0531669 C43.8271552,44.9573376 43.892336,44.86148 43.9575234,44.7655828 L44.5429346,43.9011285 C47.3268507,39.7628977 49.8240593,35.4880393 47.7482658,30.1694988 C44.3690672,21.5114095 44.6913719,18.1446262 52.8683497,7.99450751 Z M43.4168797,15.6028981 C41.2434833,17.8205255 41.7153274,24.275166 44.213695,29.9427578 C46.7120625,35.6103495 43.4168797,39.8756006 39.8854597,45.126676 C40.5019929,38.1390602 39.8587244,36.4802473 36.8588412,31.1077941 C33.858958,25.7353408 37.9685715,18.6230687 43.4168797,15.6028981 Z M64.7721431,8.03208837 C69.8550649,12.4485247 71.6576876,12.8901943 72.209291,17.4374541 C72.7608943,21.9847139 70.2967086,24.09635 71.2822168,30.0346208 C60.9002329,21.508497 69.0471936,18.4710785 64.7721431,8.03208837 Z M53.3587168,1.6350624e-05 C57.0113944,4.33013045 59.1813933,3.19675313 61.5638477,8.73089813 C63.9463021,14.2650431 60.9615342,15.5202483 63,22.1753596 C51.0029851,11.8529541 58.4461322,11.0173257 53.3587168,1.6350624e-05 Z" id="Combined-Shape-Copy" fill="#C84F46"></path>
<path d="M52.8683497,7.99450751 C48.9069025,24.0969979 69.1669662,21.1536885 69.1669662,44.7198348 C75.794917,37.1051328 70.4463322,26.8453748 81.058456,17.1725913 C81.058456,39.3863344 102.155357,38.1365645 94.0121919,69.2249916 C97.7866664,66.9118477 100.501055,63.103682 102.155357,57.8004944 C105.491398,63.103682 104.245257,72.5297487 98.416934,86.0786945 C103.547554,83.9150474 106.558165,80.9232746 108.378132,76.7271327 C109.04106,105.964869 89.5062156,126.366871 61.9867103,125.995028 C34.467205,125.623185 13.5784967,101.642443 18.2649214,71.802957 C18.9624768,74.9072731 21.5182769,78.5533566 25.1832626,80.0729101 C18.6891247,50.7321037 30.922502,49.6337285 36.0056643,33.9730813 C35.1796585,42.297059 36.0056643,46.924952 40.0208236,51.7593505 C40.7309276,49.6474958 42.0145668,47.6388953 43.3724725,45.627895 L43.7620351,45.0531669 C43.8271552,44.9573376 43.892336,44.86148 43.9575234,44.7655828 L44.5429346,43.9011285 C47.3268507,39.7628977 49.8240593,35.4880393 47.7482658,30.1694988 C44.3690672,21.5114095 44.6913719,18.1446262 52.8683497,7.99450751 Z M43.4168797,15.6028981 C41.2434833,17.8205255 41.7153274,24.275166 44.213695,29.9427578 C46.7120625,35.6103495 43.4168797,39.8756006 39.8854597,45.126676 C40.5019929,38.1390602 39.8587244,36.4802473 36.8588412,31.1077941 C33.858958,25.7353408 37.9685715,18.6230687 43.4168797,15.6028981 Z M64.7721431,8.03208837 C69.8550649,12.4485247 71.6576876,12.8901943 72.209291,17.4374541 C72.7608943,21.9847139 70.2967086,24.09635 71.2822168,30.0346208 C60.9002329,21.508497 69.0471936,18.4710785 64.7721431,8.03208837 Z M53.3587168,1.6350624e-05 C57.0113944,4.33013045 59.1813933,3.19675313 61.5638477,8.73089813 C63.9463021,14.2650431 60.9615342,15.5202483 63,22.1753596 C51.0029851,11.8529541 58.4461322,11.0173257 53.3587168,1.6350624e-05 Z" id="Combined-Shape-Copy" fill="#C84F46" />
</g>
</svg>
</template>
Expand Down