Skip to content

Conversation

@yifancong
Copy link
Contributor

@yifancong yifancong commented Nov 10, 2025

Summary

image

Related Links

Copilot AI review requested due to automatic review settings November 10, 2025 02:57
@netlify
Copy link

netlify bot commented Nov 10, 2025

Deploy Preview for rsdoctor ready!

Name Link
🔨 Latest commit 4bb0a37
🔍 Latest deploy log https://app.netlify.com/projects/rsdoctor/deploys/691e8a5a92e4630007fa2941
😎 Deploy Preview https://deploy-preview-1397--rsdoctor.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the resolver plugin implementation by moving it from webpack-specific code to a shared core module to support both Webpack and Rspack. The key change is consolidating the resolver logic into a reusable base implementation.

  • Moved InternalResolverPlugin from packages/webpack-plugin/src/plugins/resolver.ts to packages/core/src/inner-plugins/plugins/resolver.ts
  • Updated Webpack and Rspack plugins to import from the new location and use proper type parameters
  • Fixed import paths in types package to use relative imports instead of path aliases

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/webpack-plugin/src/plugins/resolver.ts Deleted the webpack-specific resolver plugin implementation
packages/webpack-plugin/src/plugin.ts Updated to import InternalResolverPlugin from core and added type parameter <Compiler>
packages/rspack-plugin/src/plugin.ts Added import and instantiation of InternalResolverPlugin with proper type parameter
packages/core/src/inner-plugins/plugins/resolver.ts Added new shared resolver plugin implementation using normalModuleFactory hooks
packages/core/src/inner-plugins/plugins/index.ts Exported the new resolver plugin
packages/types/src/sdk/instance.ts Fixed import paths from aliases to relative paths
packages/types/src/plugin/rspack.ts Added new Rspack-specific type definitions for resolver implementation
examples/rsbuild-minimal/rsbuild.config.ts Enabled resolver feature and commented out brief mode config for testing
examples/modern-minimal/modern.config.ts Added 'resolver' to features array to enable resolver functionality
Comments suppressed due to low confidence (1)

packages/rspack-plugin/src/plugin.ts:170

  • Duplicate condition checking this.options.features.resolver. The second block (lines 164-170) will always execute if the first block (lines 158-162) executes, but logs that Rspack doesn't support resolver capabilities immediately after instantiating the resolver plugin. Either remove the plugin instantiation or remove the warning message, as they contradict each other.
      if (this.options.features.resolver) {
        new InternalResolverPlugin<Plugin.BaseCompilerType<'rspack'>>(
          this,
        ).apply(compiler);
      }

      if (this.options.features.resolver) {
        logger.info(
          chalk.yellow(
            'Rspack currently does not support Resolver capabilities.',
          ),
        );
      }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yifancong yifancong force-pushed the feat/rspack-resolver branch from e11a393 to ee538d6 Compare November 11, 2025 08:40
@yifancong yifancong closed this Nov 20, 2025
@yifancong yifancong reopened this Nov 20, 2025
@yifancong yifancong force-pushed the feat/rspack-resolver branch from 4e44737 to 4bb0a37 Compare November 20, 2025 03:26
@yifancong yifancong requested a review from fi3ework November 20, 2025 06:18
@yifancong yifancong merged commit 08d67e7 into main Nov 20, 2025
9 checks passed
@yifancong yifancong deleted the feat/rspack-resolver branch November 20, 2025 06:31
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.

3 participants