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

fix: update svelte-eslint-parser to fix nested {#snippet} #805

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

baseballyama
Copy link
Member

No description provided.

Copy link

changeset-bot bot commented Jun 23, 2024

🦋 Changeset detected

Latest commit: 4f7a007

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-svelte Patch

Not sure what this means? Click here to learn what changesets are.

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

@baseballyama baseballyama force-pushed the update-parser_0_39_2 branch from 16d35d3 to f282044 Compare June 23, 2024 17:58
@ota-meshi
Copy link
Member

Hmm. The latest svelte/compiler v5 seems to give an OOM error for the following code. I don't know why yet.

{#snippet
foo(
{
a
}
)
}
{/snippet}
{@render
foo(
{
a
}
)
}

@ota-meshi
Copy link
Member

There seems to be no problem with 5.0.0-next.163. There may be a problem with the difference with 5.0.0-next.164.

@ota-meshi
Copy link
Member

ota-meshi commented Jun 23, 2024

I haven't written a test case yet, so I'm not sure, but we may need to add the s flag to handle newline characters.

https://github.com/sveltejs/svelte/pull/12070/files#diff-07a9d6689d63c982b2e3a45ae4bd624240d27824e8c5bdce8a539b4d2a7c36f1R282

-			params += parser.read(/^./);
+			params += parser.read(/^./s);

@ota-meshi
Copy link
Member

It seems to have been fixed already, we just need to wait for the next version release.

https://github.com/sveltejs/svelte/pull/12144/files

@@ -76,7 +76,9 @@ jobs:
run: pnpm install -D -w eslint@${{ matrix.eslint }}

- name: Test
run: pnpm test
run: |
export NODE_OPTIONS="--max-old-space-size=8192"
Copy link
Member

Choose a reason for hiding this comment

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

I expect this workaround will no longer be necessary once v5.0.0-165 is released.

@ota-meshi
Copy link
Member

Some issues were still not fixed, so I opened an issue in the svelte repo.

sveltejs/svelte#12156

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

Thank you!
Everything works fine with the latest version of svelte/compiler.

@ota-meshi ota-meshi merged commit 6e4d3ed into main Jun 24, 2024
13 checks passed
@ota-meshi ota-meshi deleted the update-parser_0_39_2 branch June 24, 2024 11:17
ota-meshi pushed a commit that referenced this pull request Jun 24, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## eslint-plugin-svelte@2.41.0

### Minor Changes

- [#802](#802)
[`be64d36`](be64d36)
Thanks [@ota-meshi](https://github.com/ota-meshi)! - fix: broken
indentation of if condition in `svelte/indent` rule

### Patch Changes

- [#789](#789)
[`0bc17df`](0bc17df)
Thanks [@KuSh](https://github.com/KuSh)! - chore: Use eslint types for
exported configs

- [#805](#805)
[`6e4d3ed`](6e4d3ed)
Thanks [@baseballyama](https://github.com/baseballyama)! - fix: update
`svelte-eslint-parser` to fix nested `{#snippet}`

- [#800](#800)
[`580f44f`](580f44f)
Thanks [@ota-meshi](https://github.com/ota-meshi)! - feat: add name to
flat configs.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

2 participants