feat: no-not-data-props-in-kit-pages #283
Conversation
🦋 Changeset detectedLatest commit: cde30a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
baseballyama
force-pushed
the
feat/no-not-data-props-in-kit-pages
branch
2 times, most recently
from
cfde211 to
a65362a
Compare
baseballyama
force-pushed
the
feat/no-not-data-props-in-kit-pages
branch
from
a65362a to
7a41b55
Compare
baseballyama
changed the title
ota-meshi
left a comment
ota-meshi
left a comment
There was a problem hiding this comment.
Thank you for this PR!
I made some comments.
| }, | ||
|
|
||
| // export let xxx | ||
| [`ExportNamedDeclaration > VariableDeclaration > VariableDeclarator > Identifier`]: |
There was a problem hiding this comment.
I think it's not enough to just check the Identifier.
export let {a, b} = objI think it's better to select a VariableDeclarator and report an node.id other than Identifire, or something other than data or error.
"ExportNamedDeclaration > VariableDeclaration > VariableDeclarator"
There was a problem hiding this comment.
Oh export let { data, errors } = { data: {}, errors: {} } is correct way.
https://svelte.dev/repl/61922177a5944a5294e9551787479d6a?version=3.52.0
I updated the logic, so could you please review it again?
| type: "problem", | ||
| }, | ||
| create(context) { | ||
| if (!isKitPageComponent(context)) return {} |
There was a problem hiding this comment.
I think isKitPageComponent needs to be changed to be more strict.
I think we need to check if the file name starts with +.
For filenames that don't start with +, it may just be a component.
There was a problem hiding this comment.
I added check logic and fixed tests.
| @@ -0,0 +1,6 @@ | |||
| <script> | |||
| export let data | |||
| export let errors | |||
There was a problem hiding this comment.
I saw this and noticed that errors is also allowed.
So I think it would be better to change the rule name (no-not-data-props-in-kit-pages) from what I suggested 😓.
Do you have any good ideas?
For example, how about a name like valid-prop-names-in-kit-pages?
There was a problem hiding this comment.
I changed the rule name according to your suggestion!
|
I'm sorry, could you change the namespace of settings? #290 |
2 participants
resolve: #241
This is based on #281.
So At first, we should merge #281.