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

Update docs & error messages #21

Open
quezak opened this issue Jan 31, 2019 · 1 comment
Open

Update docs & error messages #21

quezak opened this issue Jan 31, 2019 · 1 comment
Labels
docs Documentation related

Comments

@quezak
Copy link

quezak commented Jan 31, 2019

First of all, nice linter rule :)

I had some trouble configuring the rule though, here are my insights:

  • you could describe in README that the order of matching-groups serves as priorities from top to bottom, I found that out via trial-and-error and later from closed issues.
  • what does the type: 'project' | 'dependencies' option do in matching-groups? It's not mentioned in the README, my guess would be that the matching group selects only in-project imports and external deps, but my rules didn't work until I removed the type fields completely. It may have to do with @-prefixed imports -- I have two kinds of those: external ones, like @nestjs/common, and internal ones like @utils/something, configured through aliases in tsconfig-paths (or module-alias for a non-TS version).
  • the error messages for the errors with @-prefixed imports was confusing: it said This import declaration should appear in an earlier group ("namespaced-deps", number 2), while the import on the mentioned line was indeed a namespaced-deps import and it was indeed group 2 in the file. Maybe it would be useful to also describe in the message, to which group was the import actually matched?
@Gelio
Copy link
Owner

Gelio commented Feb 2, 2019

@quezak Thank you for the suggestions and feedback 😄

  • you could describe in README that the order of matching-groups serves as priorities from top to bottom, I found that out via trial-and-error and later from closed issues.

Sure, good point. Updating the README is on my todo list, as it is not ideal right now 🙂

  • what does the type: 'project' | 'dependencies' option do in matching-groups?

Another flaw of the current readme. type: 'project' is not the most fortunate name, after all. It allows using a regexp to match import declarations. type: 'dependencies' simplifies the configuration for matching dependencies from node_modules (or package.json) by finding out all the third party dependencies and creating a regexp based on those. This way the user does not have to create a regexp that would match all third party dependencies if they want to have them as a separate group.

I agree, it is not documented well currently 😄 I will try to improve the docs

  • the error messages for the errors with @-prefixed imports was confusing: it said This import declaration should appear in an earlier group ("namespaced-deps", number 2), while the import on the mentioned line was indeed a namespaced-deps import and it was indeed group 2 in the file. Maybe it would be useful to also describe in the message, to which group was the import actually matched?

Could you provide a code sample for this case, as well as the rule configuration? The import group mentioned in the parentheses is the import group that the import declaration was matched to.

The reason for such an error message is that an import that appeared before this one was matched to a group with number 3 or greater.

I cannot say much without a code sample 😄

@Gelio Gelio added the docs Documentation related label Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related
Projects
None yet
Development

No branches or pull requests

2 participants