-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Suppress eslint warnings #4953
Suppress eslint warnings #4953
Conversation
🦋 Changeset detectedLatest commit: 2fb854a The changes in this PR will be included in the next version bump. 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 |
.eslintrc.cjs
Outdated
{ | ||
files: ['packages/integrations/**/*.ts'], | ||
rules: { | ||
'no-console': 'off', |
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.
Thoughts on making this something like
'no-console': ['error', { allow: ['warn', 'error', 'info'] }],
That way we still catch random console.log
or console.table
while still allowing deliberate warn
, error
and info
?
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.
That sounds good. Looks like we're using console.log
and console.debug
at:
astro/packages/integrations/image/src/utils/logger.ts
Lines 71 to 74 in 5bcd76e
export const info = log('info', console.info); | |
export const debug = log('debug', console.debug); | |
export const warn = log('warn', console.warn); | |
export const error = log('error', console.error); |
astro/packages/integrations/mdx/src/index.ts
Lines 53 to 61 in 5bcd76e
console.log( | |
blue(`[MDX] Now inheriting remark and rehype plugins from "markdown" config.`) | |
); | |
console.log( | |
`If you applied a plugin to both your Markdown and MDX configs, we suggest ${bold( | |
'removing the duplicate MDX entry.' | |
)}` | |
); | |
console.log(`See "extendPlugins" option to configure this behavior.`); |
I guess we can have console.debug
allowed, and use line comments to suppress the second code as that's a temporary log only until mdx 1.0 (per the comment above the code)
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.
In my opinion, even considering that it's going away soon, those MDX console.log
should probably all be changed to console.info
, but either is fine really!
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.
I'll make it console.info
then 👍 would be great to get rid of the eslint comments
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.
Great work 😄
Changes
Suppress
console.log
usage warnings from eslint in integration packagesTesting
N/A. GitHub shouldn't annotate them in the UI anymore.
Docs
N/A