Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: addprint(...) function#16188

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
Rich-Harris wants to merge41 commits intomain
base:main
Choose a base branch
Loading
fromprint
Draft

feat: addprint(...) function#16188

Rich-Harris wants to merge41 commits intomainfromprint

Conversation

Rich-Harris
Copy link
Member

Over onsveltejs/esrap#68 we're working on makingesrap pluggable, so that it can be used to print any AST composed of{ type: string, ... } nodes rather than justestree and its TypeScript extensions. That includes Svelte ASTs.

The main motivation for exposing this is so that we can make it easier to write preprocessors. Historically, Svelte exposed apreprocess API, but it's all strings and duct tape, and it's difficult to integrate preprocessors cleanly with bundlers as we've seen withenhanced-img.

When the preprocessor API was introduced, things looked very different. Preprocessing was necessary to support things like TypeScript and Sass. Today, TypeScript is supported natively, and CSS is sufficiently capable that Sass is little more than a historical curiosity.

In the long term, we'd therefore like to move away from the preprocessor API in favour of providing more robust lower-level utilities. For exampleenhanced-img, which can only be used in a Vite context, really should just be a Vite plugin:

import{parse,print}from'svelte/compiler';import{walk}from'zimmerframe';functiontransform(code){constast=parse(code);consttransformed=walk(ast,null,{RegularElement(node,context){if(node.name!=='enhanced:img')return;// ...}});returnprint(ast);}

There are other potential uses, such as migrations orsv add or having a 'format' button in the playground.

As a side-effect, quality of compiler output will be slightly better in certain cases, such as when encountering comments inside nodes. (Today, we attachleadingComments andtrailingComments to each node, but this is a brittle and not-very-widely-used convention. The newesrap API expects an array ofcomments to be passed instead.)

This functionality already exists insvelte-ast-print (thank you@xeho91!), but having it in core means we can re-use theesrap version that's already installed alongsidesvelte, and will help ensure it stays current with new features.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC:https://github.com/sveltejs/rfcs
  • Prefix your PR title withfeat:,fix:,chore:, ordocs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code withinpackages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests withpnpm test and lint the project withpnpm lint

Ocean-OS, manuel3108, JReinhold, AdrianGonz97, gterras, and lishaduck reacted with hooray emojixeho91, JReinhold, and lishaduck reacted with heart emoji
@changeset-botchangeset-bot
Copy link

changeset-botbot commentedJun 17, 2025
edited
Loading

⚠️ No Changeset found

Latest commit:1377c40

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go.If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@svelte-docs-bot
Copy link

@xeho91
Copy link
Contributor

xeho91 commentedJun 17, 2025
edited
Loading

Nice!

I'll be happy to sunsetsvelte-ast-print in favour of built-in into the coreprint() function. 👍
I'm glad the need for it finally reached this point. I'm swamped right now with my real-life stuff, but just in case, both@manuel3108 and@paoloricciuti have access to thesvelte-ast-print repository in case I don't manage to announce it on time.

manuel3108 reacted with thumbs up emoji

manuel3108
manuel3108 previously approved these changesJun 21, 2025
Copy link
Member

@manuel3108manuel3108 left a comment

Choose a reason for hiding this comment

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

This already works super great.

I updated my pr forsvsveltejs/cli#568 to not only useparse but alsoprint, and i basically didn't need to change anything in comparison tosvelte-ast-print. I'm testing this with the paraglide addon, that is now printed nearly correctly, apart from the quoting issue mentioned below.

Apart from that we still need to figure out formatting, as the paraglide addon is currently printed in one line. This happens because I manually mutate the ast during the runtime and then print the ast.

I know all of this is in very early stages, but wanted to already leave some feedback

manuel3108

This comment was marked as outdated.

@github-actionsGitHub Actions
Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@16188

Copy link
Contributor

Choose a reason for hiding this comment

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

overall lgtm, but would be good to get a second pair of eyes...

"aria-query": "^5.3.1",
"axobject-query": "^4.1.0",
"clsx": "^2.1.1",
"esm-env": "^1.2.1",
"esrap": "^1.4.8",
"esrap": "https://pkg.pr.new/sveltejs/esrap@17c22f5",

Choose a reason for hiding this comment

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

are we shipping this with a pkg.pr.new dep?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

no, this PR is placeholder — it exists primarily to validate that the newesrap API makes sense

elliott-with-the-longest-name-on-github reacted with thumbs up emoji
@Rich-HarrisRich-Harris mentioned this pull requestJun 25, 2025
6 tasks
@elliott-with-the-longest-name-on-githubelliott-with-the-longest-name-on-github dismissed theirstale reviewJune 25, 2025 23:26

realized I approved instead of commenting; will re-review after it's out of draft

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@manuel3108manuel3108manuel3108 left review comments

@Ocean-OSOcean-OSOcean-OS left review comments

@elliott-with-the-longest-name-on-githubelliott-with-the-longest-name-on-githubelliott-with-the-longest-name-on-github left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@Rich-Harris@xeho91@manuel3108@Ocean-OS@elliott-with-the-longest-name-on-github

[8]ページ先頭

©2009-2025 Movatter.jp