-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[enhanced-image] support svelte 5 + fix types #12822
Changes from all commits
53f24a9
081fefe
8221b82
deabf85
6cad2f7
de2db7a
69dd692
bd1f53b
fe5becc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@sveltejs/enhanced-img": minor | ||
--- | ||
|
||
breaking: require Svelte 5 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
/** @import { AST } from 'svelte/compiler' */ | ||
|
||
import { existsSync } from 'node:fs'; | ||
import * as path from 'node:path'; | ||
|
||
import MagicString from 'magic-string'; | ||
import { asyncWalk } from 'estree-walker'; | ||
import { walk } from 'zimmerframe'; | ||
Comment on lines
-5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
import { VERSION } from 'svelte/compiler'; | ||
import { parse } from 'svelte-parse-markup'; | ||
|
||
|
@@ -32,7 +34,7 @@ export function image(opts) { | |
} | ||
|
||
const s = new MagicString(content); | ||
const ast = parse(content, { filename }); | ||
const ast = parse(content, { filename, modern: true }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its 2024 |
||
|
||
/** | ||
* Import path to import name | ||
|
@@ -49,20 +51,26 @@ export function image(opts) { | |
const consts = new Map(); | ||
|
||
/** | ||
* @param {import('svelte/types/compiler/interfaces').TemplateNode} node | ||
* @param {{ type: string, start: number, end: number, raw: string }} src_attribute | ||
* @param {import('svelte/compiler').AST.RegularElement} node | ||
* @param {AST.Text | AST.ExpressionTag} src_attribute | ||
Comment on lines
+54
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use real types |
||
* @returns {Promise<void>} | ||
*/ | ||
async function update_element(node, src_attribute) { | ||
// TODO: this will become ExpressionTag in Svelte 5 | ||
if (src_attribute.type === 'MustacheTag') { | ||
const src_var_name = content | ||
.substring(src_attribute.start + 1, src_attribute.end - 1) | ||
.trim(); | ||
s.update(node.start, node.end, dynamic_img_to_picture(content, node, src_var_name)); | ||
return; | ||
} else if (src_attribute.type === 'AttributeShorthand') { | ||
const src_var_name = content.substring(src_attribute.start, src_attribute.end).trim(); | ||
if (src_attribute.type === 'ExpressionTag') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed in the new AST. |
||
const start = | ||
'end' in src_attribute.expression | ||
? src_attribute.expression.end | ||
: src_attribute.expression.range?.[0]; | ||
const end = | ||
'start' in src_attribute.expression | ||
? src_attribute.expression.start | ||
: src_attribute.expression.range?.[1]; | ||
|
||
if (typeof start !== 'number' || typeof end !== 'number') { | ||
throw new Error('ExpressionTag has no range'); | ||
} | ||
Comment on lines
+60
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is technically more correct although I'm unsure we would ever realistically hit this issue. It was required for typescript to shut up tho. The AST relating to locations did change very slightly with v5s modern AST, using |
||
const src_var_name = content.substring(start, end).trim(); | ||
|
||
s.update(node.start, node.end, dynamic_img_to_picture(content, node, src_var_name)); | ||
return; | ||
} | ||
|
@@ -73,10 +81,10 @@ export function image(opts) { | |
const sizes = get_attr_value(node, 'sizes'); | ||
const width = get_attr_value(node, 'width'); | ||
url += url.includes('?') ? '&' : '?'; | ||
if (sizes) { | ||
if (sizes && 'raw' in sizes) { | ||
url += 'imgSizes=' + encodeURIComponent(sizes.raw) + '&'; | ||
} | ||
if (width) { | ||
if (width && 'raw' in width) { | ||
Comment on lines
+84
to
+87
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typescript |
||
url += 'imgWidth=' + encodeURIComponent(width.raw) + '&'; | ||
} | ||
url += 'enhanced'; | ||
|
@@ -119,23 +127,35 @@ export function image(opts) { | |
} | ||
} | ||
|
||
/** | ||
* @type {Array<ReturnType<typeof update_element>>} | ||
*/ | ||
const pending_ast_updates = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To store promises. |
||
// TODO: switch to zimmerframe with Svelte 5 | ||
// @ts-ignore | ||
await asyncWalk(ast.html, { | ||
/** | ||
* @param {import('svelte/types/compiler/interfaces').TemplateNode} node | ||
*/ | ||
async enter(node) { | ||
if (node.type === 'Element') { | ||
// Compare node tag match | ||
if (node.name === 'enhanced:img') { | ||
|
||
walk( | ||
/** @type {import('svelte/compiler').AST.Root} */ (ast), | ||
{}, | ||
{ | ||
_(_, { next }) { | ||
next(); | ||
}, | ||
/** @param {import('svelte/compiler').AST.RegularElement} node */ | ||
// @ts-ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot for the life of me get this to understand me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the trick is for the first argument to |
||
RegularElement(node) { | ||
if ('name' in node && node.name === 'enhanced:img') { | ||
// Compare node tag match | ||
const src = get_attr_value(node, 'src'); | ||
if (!src) return; | ||
await update_element(node, src); | ||
|
||
if (!src || typeof src === 'boolean') return; | ||
|
||
pending_ast_updates.push(update_element(node, src)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
} | ||
}); | ||
); | ||
|
||
await Promise.all(pending_ast_updates); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We make sure all node modifications have been completed before proceeding. |
||
|
||
// add imports and consts to <script module> block | ||
let text = ''; | ||
|
@@ -215,22 +235,31 @@ export function parseObject(str) { | |
} | ||
|
||
/** | ||
* @param {import('svelte/types/compiler/interfaces').TemplateNode} node | ||
* @param {import('../types/internal.ts').TemplateNode} node | ||
* @param {string} attr | ||
* @returns {AST.Text | AST.ExpressionTag | undefined} | ||
*/ | ||
function get_attr_value(node, attr) { | ||
if (!('type' in node) || !('attributes' in node)) return; | ||
const attribute = node.attributes.find( | ||
/** @param {any} v */ (v) => v.type === 'Attribute' && v.name === attr | ||
); | ||
|
||
if (!attribute) return; | ||
if (!attribute || !('value' in attribute) || typeof attribute.value === 'boolean') return; | ||
|
||
return attribute.value[0]; | ||
// Check if value is an array and has at least one element | ||
if (Array.isArray(attribute.value)) { | ||
if (attribute.value.length > 0) return attribute.value[0]; | ||
return; | ||
} | ||
|
||
// If it's not an array or is empty, return the value as is | ||
return attribute.value; | ||
} | ||
|
||
/** | ||
* @param {string} content | ||
* @param {Array<import('svelte/types/compiler/interfaces').BaseDirective | import('svelte/types/compiler/interfaces').Attribute | import('svelte/types/compiler/interfaces').SpreadAttribute>} attributes | ||
* @param {import('../types/internal.ts').Attribute[]} attributes | ||
* @param {{ | ||
* src: string, | ||
* width: string | number, | ||
|
@@ -239,7 +268,7 @@ function get_attr_value(node, attr) { | |
*/ | ||
function serialize_img_attributes(content, attributes, details) { | ||
const attribute_strings = attributes.map((attribute) => { | ||
if (attribute.name === 'src') { | ||
if ('name' in attribute && attribute.name === 'src') { | ||
return `src=${details.src}`; | ||
} | ||
return content.substring(attribute.start, attribute.end); | ||
|
@@ -250,8 +279,13 @@ function serialize_img_attributes(content, attributes, details) { | |
/** @type {number | undefined} */ | ||
let user_height; | ||
for (const attribute of attributes) { | ||
if (attribute.name === 'width') user_width = parseInt(attribute.value[0].raw); | ||
if (attribute.name === 'height') user_height = parseInt(attribute.value[0].raw); | ||
if ('name' in attribute && 'value' in attribute) { | ||
const value = Array.isArray(attribute.value) ? attribute.value[0] : attribute.value; | ||
if (typeof value === 'object' && 'raw' in value) { | ||
if (attribute.name === 'width') user_width = parseInt(value.raw); | ||
if (attribute.name === 'height') user_height = parseInt(value.raw); | ||
} | ||
} | ||
} | ||
if (!user_width && !user_height) { | ||
attribute_strings.push(`width=${details.width}`); | ||
|
@@ -283,13 +317,15 @@ function stringToNumber(param) { | |
/** | ||
* @param {Map<string,string>} consts | ||
* @param {string} content | ||
* @param {import('svelte/types/compiler/interfaces').TemplateNode} node | ||
* @param {import('svelte/compiler').AST.RegularElement} node | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More specific type, we only pass elements to this fn. |
||
* @param {import('vite-imagetools').Picture} image | ||
*/ | ||
function img_to_picture(consts, content, node, image) { | ||
/** @type {Array<import('svelte/types/compiler/interfaces').BaseDirective | import('svelte/types/compiler/interfaces').Attribute | import('svelte/types/compiler/interfaces').SpreadAttribute>} attributes */ | ||
/** @type {import('../types/internal.ts').Attribute[]} attributes */ | ||
const attributes = node.attributes; | ||
const index = attributes.findIndex((attribute) => attribute.name === 'sizes'); | ||
const index = attributes.findIndex( | ||
(attribute) => 'name' in attribute && attribute.name === 'sizes' | ||
); | ||
let sizes_string = ''; | ||
if (index >= 0) { | ||
sizes_string = ' ' + content.substring(attributes[index].start, attributes[index].end); | ||
|
@@ -324,19 +360,21 @@ function to_value(consts, src) { | |
} | ||
return `{${var_name}}`; | ||
} | ||
|
||
return `"${src}"`; | ||
} | ||
|
||
/** | ||
* For images like `<img src={manually_imported} />` | ||
* @param {string} content | ||
* @param {import('svelte/types/compiler/interfaces').TemplateNode} node | ||
* @param {import('svelte/compiler').AST.RegularElement} node | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as sbove. |
||
* @param {string} src_var_name | ||
*/ | ||
function dynamic_img_to_picture(content, node, src_var_name) { | ||
/** @type {Array<import('svelte/types/compiler/interfaces').BaseDirective | import('svelte/types/compiler/interfaces').Attribute | import('svelte/types/compiler/interfaces').SpreadAttribute>} attributes */ | ||
const attributes = node.attributes; | ||
const index = attributes.findIndex((attribute) => attribute.name === 'sizes'); | ||
const index = attributes.findIndex( | ||
(attribute) => 'name' in attribute && attribute.name === 'sizes' | ||
); | ||
let sizes_string = ''; | ||
if (index >= 0) { | ||
sizes_string = ' ' + content.substring(attributes[index].start, attributes[index].end); | ||
|
@@ -350,7 +388,7 @@ function dynamic_img_to_picture(content, node, src_var_name) { | |
}; | ||
|
||
return `{#if typeof ${src_var_name} === 'string'} | ||
<img ${serialize_img_attributes(content, node.attributes, details)} /> | ||
<img ${serialize_img_attributes(content, attributes, details)} /> | ||
{:else} | ||
<picture> | ||
{#each Object.entries(${src_var_name}.sources) as [format, srcset]} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
const src = manual_image1; | ||
const images = [manual_image1, manual_image2]; | ||
const get_image = (image_key: number) => images[image_key]; | ||
|
||
let foo: string = 'bar'; | ||
</script> | ||
|
@@ -28,7 +29,11 @@ | |
alt="sizes test" | ||
/> | ||
|
||
<enhanced:img src="./dev.png" on:click={(foo = 'clicked an image!')} alt="event handler test" /> | ||
<enhanced:img | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update the onclick to be correct and use v5 syntax. |
||
src="./dev.png" | ||
onclick={() => (foo = 'clicked an image!')} | ||
alt="event handler test" | ||
/> | ||
|
||
<enhanced:img src="$lib/dev.png" alt="alias test" /> | ||
|
||
|
@@ -42,6 +47,10 @@ | |
<enhanced:img src={image} alt="opt-in test" /> | ||
{/each} | ||
|
||
{#each images as _, i} | ||
<enhanced:img src={get_image(i)} alt="opt-in test" /> | ||
{/each} | ||
|
||
Comment on lines
+50
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra test to make sure we can handle things that aren't just identifiers. |
||
<picture> | ||
<source src="./dev.avif" /> | ||
<source srcset="./dev.avif 500v ./bar.avif 100v" /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<script context="module"> | ||
<script module> | ||
import __IMPORTED_ASSET_0__ from "./foo.svg"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cheeky |
||
const __DECLARED_ASSET_0__ = "__VITE_ASSET__2AM7_y_a__ 1440w, __VITE_ASSET__2AM7_y_b__ 960w"; | ||
const __DECLARED_ASSET_1__ = "__VITE_ASSET__2AM7_y_c__ 1440w, __VITE_ASSET__2AM7_y_d__ 960w"; | ||
|
@@ -13,6 +13,7 @@ | |
|
||
const src = manual_image1; | ||
const images = [manual_image1, manual_image2]; | ||
const get_image = (image_key: number) => images[image_key]; | ||
|
||
let foo: string = 'bar'; | ||
</script> | ||
|
@@ -33,7 +34,7 @@ | |
|
||
<picture><source srcset="/1 1440w, /2 960w" sizes="(min-width: 60rem) 80vw, (min-width: 40rem) 90vw, 100vw" type="image/avif" /><source srcset="/3 1440w, /4 960w" sizes="(min-width: 60rem) 80vw, (min-width: 40rem) 90vw, 100vw" type="image/webp" /><source srcset="5 1440w, /6 960w" sizes="(min-width: 60rem) 80vw, (min-width: 40rem) 90vw, 100vw" type="image/png" /><img src="/7" alt="sizes test" width=1440 height=1440 /></picture> | ||
|
||
<picture><source srcset="/1 1440w, /2 960w" type="image/avif" /><source srcset="/3 1440w, /4 960w" type="image/webp" /><source srcset="5 1440w, /6 960w" type="image/png" /><img src="/7" on:click={(foo = 'clicked an image!')} alt="event handler test" width=1440 height=1440 /></picture> | ||
<picture><source srcset="/1 1440w, /2 960w" type="image/avif" /><source srcset="/3 1440w, /4 960w" type="image/webp" /><source srcset="5 1440w, /6 960w" type="image/png" /><img src="/7" onclick={() => (foo = 'clicked an image!')} alt="event handler test" width=1440 height=1440 /></picture> | ||
|
||
<picture><source srcset="/1 1440w, /2 960w" type="image/avif" /><source srcset="/3 1440w, /4 960w" type="image/webp" /><source srcset="5 1440w, /6 960w" type="image/png" /><img src="/7" alt="alias test" width=1440 height=1440 /></picture> | ||
|
||
|
@@ -65,6 +66,19 @@ | |
{/if} | ||
{/each} | ||
|
||
{#each images as _, i} | ||
{#if typeof get_image(i) === 'string'} | ||
<img src={get_image(i).img.src} alt="opt-in test" width={get_image(i).img.w} height={get_image(i).img.h} /> | ||
{:else} | ||
<picture> | ||
{#each Object.entries(get_image(i).sources) as [format, srcset]} | ||
<source {srcset} type={'image/' + format} /> | ||
{/each} | ||
<img src={get_image(i).img.src} alt="opt-in test" width={get_image(i).img.w} height={get_image(i).img.h} /> | ||
</picture> | ||
{/if} | ||
{/each} | ||
|
||
<picture> | ||
<source src="./dev.avif" /> | ||
<source srcset="./dev.avif 500v ./bar.avif 100v" /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import type { AST } from 'svelte/compiler'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just some helpful types for internal use. |
||
|
||
type ElementLike = | ||
| AST.Component | ||
| AST.TitleElement | ||
| AST.SlotElement | ||
| AST.RegularElement | ||
| AST.SvelteBody | ||
| AST.SvelteComponent | ||
| AST.SvelteDocument | ||
| AST.SvelteElement | ||
| AST.SvelteFragment | ||
| AST.SvelteHead | ||
| AST.SvelteOptionsRaw | ||
| AST.SvelteSelf | ||
| AST.SvelteWindow; | ||
|
||
type Tag = AST.ExpressionTag | AST.HtmlTag | AST.ConstTag | AST.DebugTag | AST.RenderTag; | ||
|
||
type Directive = | ||
| AST.AnimateDirective | ||
| AST.BindDirective | ||
| AST.ClassDirective | ||
| AST.LetDirective | ||
| AST.OnDirective | ||
| AST.StyleDirective | ||
| AST.TransitionDirective | ||
| AST.UseDirective; | ||
|
||
type Block = AST.EachBlock | AST.IfBlock | AST.AwaitBlock | AST.KeyBlock | AST.SnippetBlock; | ||
|
||
export type TemplateNode = AST.Text | Tag | ElementLike | AST.Comment | Block; | ||
|
||
export type Attribute = AST.Attribute | AST.SpreadAttribute | Directive; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required to support top-level jsdoc type imports.