Skip to content

fix: modify the criteria for verifying the Rsdoctor plugin#2992

Merged
chenjiahan merged 1 commit intomainfrom
fix/rsdoctor-plugin
Jul 23, 2024
Merged

fix: modify the criteria for verifying the Rsdoctor plugin#2992
chenjiahan merged 1 commit intomainfrom
fix/rsdoctor-plugin

Conversation

@yifancong
Copy link
Copy Markdown
Contributor

Summary

fix: modify the criteria for verifying the Rsdoctor plugin.

Related Links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@netlify
Copy link
Copy Markdown

netlify bot commented Jul 23, 2024

Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit b221b96
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/669f264920ae64000809b3e0
😎 Deploy Preview https://deploy-preview-2992--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 68 (🔴 down 19 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link
Copy Markdown

netlify bot commented Jul 23, 2024

Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit 248a059
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/669f6fde8b1c2c000882dbda
😎 Deploy Preview https://deploy-preview-2992--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 88 (🟢 up 11 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@yifancong yifancong force-pushed the fix/rsdoctor-plugin branch 2 times, most recently from 0fafa80 to 8b3a132 Compare July 23, 2024 07:00
@yifancong yifancong requested a review from chenjiahan July 23, 2024 07:30

let isAutoRegister = false;

function isRsdoctorPlugin(plugin: Configuration['plugins'] & { isRsdoctorPlugin?: boolean }) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function -> const + arrow function

(plugin) => plugin?.constructor?.name === pluginName,
// If user has add Rsdoctor plugin in config file, will return.
const registered = config.plugins?.some(
(plugin) => isRsdoctorPlugin(plugin as unknown as Configuration['plugins'] & { isRsdoctorPlugin?: boolean }) || plugin?.constructor?.name === pluginName,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move the plugin?.constructor?.name === pluginName to isRsdoctorPlugin() helper.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move the plugin?.constructor?.name === pluginName to isRsdoctorPlugin() helper.

@chenjiahan I just worry about remove the plugin?.constructor?.name === pluginName may be a break change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean move the logic to the function, not to remove it.

(plugin) => plugin?.constructor?.name === pluginName,
// If user has add Rsdoctor plugin in config file, will return.
const registered = config.plugins?.some(
(plugin) => isRsdoctorPlugin(plugin as unknown as Configuration['plugins'] & { isRsdoctorPlugin?: boolean }) || plugin?.constructor?.name === pluginName,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can declare a type to reuse it

Suggested change
(plugin) => isRsdoctorPlugin(plugin as unknown as Configuration['plugins'] & { isRsdoctorPlugin?: boolean }) || plugin?.constructor?.name === pluginName,
type MaybeRsdoctorPlugin = Configuration['plugins'] & { isRsdoctorPlugin?: boolean })

@yifancong yifancong force-pushed the fix/rsdoctor-plugin branch 2 times, most recently from 6e9fedf to 5743aac Compare July 23, 2024 08:06
fix: rsdoctor plugin

fix: rsdoctor plugin

feat: support add plugins for specified environment (#2986)

Co-authored-by: neverland <jait.chen@foxmail.com>

Update packages/core/src/plugins/rsdoctor.ts

Co-authored-by: neverland <chenjiahan.jait@bytedance.com>

Update packages/core/src/plugins/rsdoctor.ts

Co-authored-by: neverland <chenjiahan.jait@bytedance.com>

Update packages/core/src/plugins/rsdoctor.ts

Co-authored-by: neverland <chenjiahan.jait@bytedance.com>

fix: modify the criteria for verifying the Rsdoctor plugin.

fix: modify the criteria for verifying the Rsdoctor plugin.
@yifancong yifancong force-pushed the fix/rsdoctor-plugin branch from 5743aac to 248a059 Compare July 23, 2024 08:54
@yifancong yifancong requested a review from chenjiahan July 23, 2024 09:04
Copy link
Copy Markdown
Member

@chenjiahan chenjiahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@chenjiahan chenjiahan changed the title fix: modify the criteria for verifying the Rsdoctor plugin. fix: modify the criteria for verifying the Rsdoctor plugin Jul 23, 2024
@chenjiahan chenjiahan merged commit 6da31c0 into main Jul 23, 2024
@chenjiahan chenjiahan deleted the fix/rsdoctor-plugin branch July 23, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants