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

Support await destructuring #83

Merged
merged 8 commits into from
May 5, 2020

Conversation

xla
Copy link
Contributor

@xla xla commented Mar 22, 2020

Svelte 3.20.0 introduced support for await destructing, see sveltejs/svelte#1851 and sveltejs/svelte#4548. This change supports this syntax and keep this plugin functional with the newest svelte version.

Svelte 3.20.0 introduced support for await destructing, see
svetlejs/svelte#1851 and sveltejs/svelte#4548. This change supports
this syntax and keep this plugin functional with the newest svelte
version
Copy link

@rudolfs rudolfs left a comment

Choose a reason for hiding this comment

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

Awesome job!! 🎉

@rudolfs
Copy link

rudolfs commented Mar 23, 2020

Lol, approved the wrong PR...

@andyhot
Copy link

andyhot commented Apr 10, 2020

First of all thanks for this!

prettier-plugin-svelte currently messes up the #await syntax on projects based on svelte > 3.20.

I have code like {#await orderPayments then payments} that gets turned into {#await orderPayments then } - and even {:then terms} becomes {:then [object Object]}

I've tested with svelte 3.12 to 3.19 and seen that prettier-plugin-svelte works fine there.

The problem i see with this PR is that users of svelte < 3.20 will be affected (their templates will be broken). You can notice this if you apply the PR but downgrade svelte to 3.19 - you'd get test failures.

So, I'm wondering if a) there should be a table documenting which svelte versions are supported by each prettier-plugin-svelte release, or b) if it's possible to modify the PR so that it works in all cases (i.e. both pre and post svelte 3.20)

@aredridel
Copy link

Oof. this is biting me too.

@andyhot
Copy link

andyhot commented Apr 22, 2020

@aredridel you can add something like "prettier-plugin-svelte": "github:radicle-dev/prettier-plugin-svelte#xla/support-await-destruct" to your dev dependencies till this is incorporated here

@Conduitry
Copy link
Member

I haven't tried it yet, but I think this could be made to work with the AST from both older and newer versions of Svelte by making the expandNode function check whether node is a string and if so returning ' ' + node.

@Conduitry
Copy link
Member

The appears to be working. With that change on top of this PR, and using an older version of Svelte, all of the tests pass apart from the ones that using await destructuring (which fail because Svelte can't parse them) - and with that change and using the latest version of Svelte, all these tests still pass.

@xla Can you enable 'allow edits from maintainers' please so that I can push merge resolutions and other fixes to this branch - and won't have to open a new PR and you'll continue to get acknowledgment for your work when this is squashed?

@xla
Copy link
Contributor Author

xla commented May 5, 2020

@Conduitry I don't see that option in the sidebar. Any pointers how to enable it?

@Conduitry
Copy link
Member

Oh huh apparently this is only available for PRs opened from a fork that belongs to PR opener, and not if it belongs to an org, even one the PR opener is an owner of. That's unfortunate.

@xla
Copy link
Contributor Author

xla commented May 5, 2020

I can ammend changes directly, or invite you as a collaborator on the repo directly. Lemme know which one you prefer @Conduitry

@Conduitry
Copy link
Member

There were a number of changes, so I'd prefer to be added as a collaborator if you don't mind. Thanks!

@xla
Copy link
Contributor Author

xla commented May 5, 2020

@Conduitry Invite is out. Let me know if I can help in any way to get this done.

@Conduitry Conduitry force-pushed the xla/support-await-destruct branch from c2d961d to 8a5e528 Compare May 5, 2020 10:40
@Conduitry Conduitry merged commit 7d9f6af into sveltejs:master May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants