Not working cross-file

[jkettmann]

First of all, thanks for the great package. Should have way more stars imo.

Unfortunately, I ran into a problem. I like to keep my styled components in a separate file like MyComponent.style.js. In this case, the linter doesn't give an error. Having the styles in the same file as the component works fine though.

I'd be happy to contribute but I'd like your expert opinion first whether the linting rules can work cross-file or not.

[brendanmorrell]

Hey, Johannes. Thanks for the comment, and glad you're liking the plugin! As for linting imported components, I think it's a great idea, and it's actually been at the top of my to-do list for a while. I think it probably is possible. I personally have only used eslint in the context of parsing a single file's abstract syntax tree, but it must be possible to hit multiple files as the rule for detecting dependency cycles does just that. If you'd like to take a stab at it, that would be great, and that's where I'd start (looking at the eslint 'import/no-cycle' rule, and seeing how they use the AST parser to traverse multiplet files). Basically, we'd need a way to have the eslint ast parser follow the imports in one file to wherever the are defined in the other file, and then use that initial node to run all the logic I built in the plugin. I can explain more about how that works if necessary, but the main part of the task is just figuring out how to get eslint to follow the imports/exports to the initial node where they are defined. Let me know if you need help or more info, and good luck! (Also, I will try to add that functionality at some point even if you don't. However, I've been meaning to for some time and I'm not sure when I'll get around to it, so it may take a while)

…

On Wed, Sep 2, 2020, 08:46 Johannes ***@***.***> wrote: First of all, thanks for the great package. Should have way more stars imo. Unfortunately, I ran into a problem. I like to keep my styled components in a separate file like MyComponent.style.js. In this case, the linter doesn't give an error. Having the styles in the same file as the component works fine though. I'd be happy to contribute but I'd like your expert opinion first whether the linting rules can work cross-file or not. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADRNADNDXMIEUUUWTX3OU73SDY5CVANCNFSM4QS64GLA> .

[artyomtrityak]

That would be great to enable eslint-plugin-styled-components-a11y for this use case.

[artyomtrityak]

@jkettmann @brendanmorrell do you have a timeline/status for this work, maybe I can help too.

[jkettmann]

Unfortunately, I don't have any time in the near future

[brendanmorrell]

I actually looked into this two weeks ago for an hour or two. My initial thought on how to do it may not be possible to the degree of accuracy I would have liked, unfortunately. I haven't found a particularly good way of actually using eslint to parse files outside of the current file scope, although there very well may be a good way to do this. I think it would be doable to have a very basic version, which maybe would cover most use cases, but would certainly fail for complex ones. The basic version I'm thinking of seems kind of hacky, and less than ideal, but it would work like so:

Currently, the way this plugin works is it creates a dictionary of all of the variables created from a call to either styled. or styled(. It then grabs any of the necessary information from the attrs calls etc, and waits until the parser runs into the same name as in the dictionary when used in a jsx element, and basically copies over some info to that node and runs the linting rules on it as though it were just the original html tag with the added attributes.

The basic version of how I could see this working cross-file would be to add a parser function to run on all of the import statements in the file, just use node to look at all of those files and find the original definition of the variable being imported and check if it were a styled component, and if so, parse that in the same way we do within the original file and add that information to the styled-components dictionary.

Unfortunately, this is kind of messy and leaves a ton of edge cases (reexporting of variables such that you would need to trace this back a potentially infinite number of files, name changes of the variables at any point in any of the files which would confuse things, etc.). However, I would guess 99% of the use cases would probably be covered with just a simple version where the name never changes and it is only exported a single time and imported directly.

Having said that, not sure when I will be able to take a look. I would guess I could take another crack at it this weekend, but doubt I will have enough time to actually finish it, although I can try.

If you would like to try, you could get most of the way there without ever even really needing to understand all the logic I have in here already (although you're welcome to take a look and try to implement the whole thing if you'd like). Basically, you'd just need to create a parsing rule for import statements, and then from there, grab the variables, find the source file, and then use something like node's fs to read that file and try to have eslint parse that and grab out the appropriate variable declaration, and add it to the dictionary if it's a styled component. It's a little more than that, but that would be the main bulk of the work. The rest is just hooking it up to the parsers I have in place. If you'd like to start/add the import parsing function in this plugin, the file to do so would be collectStyledComponentsData.js. You'd just be adding the import method into that exported object. If you make any progress, that'd be great, and I'd love to hear about it.

If not, I will try to take another look, but I don't have a ton of time so it's hard to give a real timeline.

Hope that helps, and good luck!

[zdhickman]

+1 on this, I took a look into this but I need to spend more time learning more about ESLint internals before I can take a proper go at it. Our team found other ways to check a11y violations in our project, but it would be great to leverage this plugin for cross-file linting as well 😄

[brendanmorrell]

It definitely is something that would be very useful. I looked into it briefly in relation to the dependency cycle detection in eslint plugin imports, but the use case appeared different enough that I'm not sure the same method could be used. Would definitely like to figure out a way to make it work though. I'll try and find some time to figure it out soon, but if anyone has an idea of how to carry over context between files in eslint, it would be greatly appreciated.

…

On Mon, Jan 31, 2022, 14:14 Zack ***@***.***> wrote: +1 on this, I took a look into this but I need to spend more time learning more about ESLint internals before I can take a proper go at it. Our team found other ways to check a11y violations in our project, but it great to leverage this plugin for cross-file linting as well 😄 — Reply to this email directly, view it on GitHub <#9 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADRNADPPUP4AV5NH26M5KBLUY4CTFANCNFSM4QS64GLA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: <brendanmorrell/eslint-plugin-styled-components-a11y/issues/9/1026265102@ github.com>