Skip to content

add svelte/no-extra-reactive-curlies rule #201

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

Merged
merged 11 commits into from
Jul 31, 2022

Conversation

tivac
Copy link
Collaborator

@tivac tivac commented Jul 28, 2022

Porting over a simple rule from https://github.com/tivac/eslint-plugin-svelte/ as per the conversation in #198

I haven't been able to test that the documentation looks right, yarn svelte-kit is doing some funny path stuff on my windows machine. 🤷🏻‍♂️

@tivac tivac changed the title add "no-unnecessary-reactive-curlies" rule add svelte/no-unnecessary-reactive-curlies rule Jul 28, 2022
@ota-meshi
Copy link
Member

What do you think about rename the rule to no-lone-reactive-blocks or no-extra-reactive-curlies? I think it's similar to eslint's no-lone-blocks and no-extra-parens.
https://eslint.org/docs/latest/rules/no-lone-blocks
https://eslint.org/docs/latest/rules/no-extra-parens

@ota-meshi
Copy link
Member

I didn't explain it, use the yarn docs:watch command to preview the document site. However, I'm not sure if there is a problem with windows.

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 for this PR!
I have some change requests.

@tivac
Copy link
Collaborator Author

tivac commented Jul 29, 2022

I didn't explain it, use the yarn docs:watch command to preview the document site. However, I'm not sure if there is a problem with windows.

This is what I see as output

C:\Users\pcavi\Documents\GitHub\eslint-svelte>yarn docs:watch
yarn run v1.22.19
$ yarn svelte-kit dev
$ node --experimental-loader ./svelte-kit-import-hook.mjs node_modules/vite/bin/vite.js dev
(node:9368) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9368) DeprecationWarning: Obsolete loader hook(s) supplied and will be ignored: transformSource
build@ \C:\Users\pcavi\Documents\GitHub\eslint-svelte\docs-svelte-kit\build-system\src\eslint.mjs
X [ERROR] Could not read from file: C:\Users\pcavi\Documents\GitHub\eslint-svelte\C:\Users\pcavi\Documents\GitHub\eslint-svelte\docs-svelte-kit\build-system\src\process-shim.mjs

X [ERROR] Could not resolve "\\C:\\Users\\pcavi\\Documents\\GitHub\\eslint-svelte\\docs-svelte-kit\\build-system\\src\\eslint.mjs"

failed to load config from C:\Users\pcavi\Documents\GitHub\eslint-svelte\vite.config.mts
error when starting dev server:
Error: Build failed with 2 errors:
error: Could not read from file: C:\Users\pcavi\Documents\GitHub\eslint-svelte\C:\Users\pcavi\Documents\GitHub\eslint-svelte\docs-svelte-kit\build-system\src\process-shim.mjs
error: Could not resolve "\\C:\\Users\\pcavi\\Documents\\GitHub\\eslint-svelte\\docs-svelte-kit\\build-system\\src\\eslint.mjs"
    at failureErrorWithLog (C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:1621:15)
    at C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:1263:28
    at runOnEndCallbacks (C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:1176:65)
    at buildResponseToResult (C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:1261:7)
    at C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:1374:14
    at C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:675:9
    at handleIncomingPacket (C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:772:9)
    at Socket.readFromStdout (C:\Users\pcavi\Documents\GitHub\eslint-svelte\node_modules\esbuild\lib\main.js:641:7)
    at Socket.emit (node:events:527:28)
    at addChunk (node:internal/streams/readable:315:12)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

This one in particular seems like a likely culprit with the doubled-up folder structure.

X [ERROR] Could not read from file: C:\Users\pcavi\Documents\GitHub\eslint-svelte\C:\Users\pcavi\Documents\GitHub\eslint-svelte\docs-svelte-kit\build-system\src\process-shim.mjs

@tivac tivac requested a review from ota-meshi July 29, 2022 06:27
@tivac tivac changed the title add svelte/no-unnecessary-reactive-curlies rule add svelte/no-extra-reactive-curlies rule Jul 29, 2022
@ota-meshi
Copy link
Member

This one in particular seems like a likely culprit with the doubled-up folder structure.

Hmm. I don't know how to fix it. I think we giving esbuild the correct absolute path.

@tivac tivac force-pushed the no-unnecessary-reactive-curlies branch from 3d9ebd6 to 26bc92d Compare July 31, 2022 02:30
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.

LGTM! Thank you very much

@ota-meshi ota-meshi merged commit 6b04778 into sveltejs:main Jul 31, 2022
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