-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
feat(v2): Allow plugins to consume webpack stats #4021
Conversation
Signed-off-by: Reece Dunham <me@rdil.rocks>
✔️ Deploy preview for docusaurus-2 ready! 🔨 Explore the source changes: 96a4232 🔍 Inspect the deploy logs: https://app.netlify.com/sites/docusaurus-2/deploys/5ffc5cdef3a490000784e920 😎 Browse the preview: https://deploy-preview-4021--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4021--docusaurus-2.netlify.app/classic/ |
Size Change: 0 B Total Size: 26.7 kB ℹ️ View Unchanged
|
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.
LGTM, thanks 👍
Was wondering why the start.ts type is updated?
@@ -103,7 +103,7 @@ export default async function start( | |||
cwd: siteDir, | |||
ignoreInitial: true, | |||
usePolling: !!cliOptions.poll, | |||
interval: Number.isInteger(cliOptions.poll) | |||
interval: Number.isInteger((cliOptions.poll as unknown) as never) |
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.
what was the problem?
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 thought I was going to need to update this for this PR, and my IDE was complaining about it. I'll revert if you want.
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.
As it's not strictly related to the PR and I don't understand the intent, I'd prefer removing this line and discuss it as a separate issue/PR if needed
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.
Sure.
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.
Done.
console.error(e); | ||
}); | ||
reject(new Error('Failed to compile with errors.')); | ||
} | ||
if (stats?.hasWarnings()) { | ||
// Custom filtering warnings (see https://github.com/webpack/webpack/issues/7841). | ||
let {warnings} = stats.toJson('errors-warnings'); | ||
let warnings = [...allStats.warnings]; |
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.
are there cases where the warning array is not defined?
Maybe we could replace the condition with if (allStats.warnings?.length > 0)
, and TS would understand better that the array is there or something?
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 there so the original stats.warnings
doesn't get modified.
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 see thanks.
It shouldn't be, but won't harm to keep this code
thanks 👍 |
No, thank you! My plugin is now a little closer to completion 😄 |
great to know 👍 add it to the community section when ready |
Signed-off-by: Reece Dunham me@rdil.rocks
Motivation
I need to use one of these stats in my own plugin, so it was
patch-package
, or do this, which gives plugins a LOT more options for post-processing and other capabilities.Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
My test plugin worked against the branch.
Related PRs
n/a