-
Notifications
You must be signed in to change notification settings - Fork 61
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
no-uninstalled-addons: support satisfies operator and default reexport #139
no-uninstalled-addons: support satisfies operator and default reexport #139
Conversation
5844b07
to
783542f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @hjoelh thank you so much for your contribution! I added a comment for you to check out
lib/rules/no-uninstalled-addons.ts
Outdated
} | ||
}, | ||
ExportDefaultDeclaration: function (node) { | ||
if (isIdentifier(node.declaration)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a getMetaObjectExpression
utility function that analyzes a default export node and gives a valid meta object, and that supports all the notations, alongside their TS variants:
- default export {}
- const meta; export default meta;
I'm so sorry that you spent time in this without knowing it! But could you replace the implementation with something like this (and keep the tests):
import { getMetaObjectExpression } from '../utils'
// ...
ExportDefaultDeclaration: function (node) {
const meta = getMetaObjectExpression(node, context)
if (!meta) {
return null
}
const addonsProp = meta.properties.find(
(prop): prop is TSESTree.Property =>
isProperty(prop) && isIdentifier(prop.key) && prop.key.name === 'addons'
)
if (addonsProp && addonsProp.value && isArrayExpression(addonsProp.value)) {
reportUninstalledAddons(addonsProp.value)
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @yannbf! No worries at all, I'm not too familiar with ASTs and eslint plugins - thats a nice util 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bump @yannbf 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just perfect! Thank you so so much @hjoelh <3
🚀 PR was released in |
Issue: #
What Changed
no-uninstalled-addons now works with 2 additional export ways
(as shown in the docs)
&
Checklist
Check the ones applicable to your change:
yarn update-all
Change Type
Indicate the type of change your pull request is:
maintenance
documentation
patch
minor
major