-
Notifications
You must be signed in to change notification settings - Fork 11.9k
refactor: address issue with bazel resolution of peer dependencies (main) #31782
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
Conversation
Key changes include:
- **Removal of PNPM Hook:** The `.pnpmfile.cjs` file, which previously converted peer dependencies into
direct dependencies, has been deleted. This workaround is no longer necessary with the adoption of dynamic
imports. The `MODULE.bazel` file has been updated to remove references to it.
- **Dynamic Imports**
- In the `build_webpack` package, static `import` statements for `webpack` and `webpack-dev-server` are
replaced with dynamic `import()` calls. This defers their loading until they are actively required during
the process.
- In the `ssr-dev-server`, `browser-sync` is now loaded using `createRequire` relative to the workspace
root, improving resolution reliability.
- **Build Configuration:** Associated `BUILD.bazel` files have been updated to remove dependency declarations that are now obsolete due to the new lazy-loading approach.
| let browserSync: typeof import('browser-sync'); | ||
| try { | ||
| browserSync = require('browser-sync'); | ||
| browserSync = createRequire(context.workspaceRoot + '/')('browser-sync'); |
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.
browser-sync is installed in the project (app), and thus the resolution should happen from the workspaceRoot not from this package.
| } | ||
| } else { | ||
| return of(webpack(c)); | ||
| return from(import('webpack').then((mod) => mod.default(c))); |
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.
When a factory is not provided we should need to try to resolve webpack from this directory. This will fail when using the peerDeps of build-angular due to how modules are hoisted.
|
This PR was merged into the repository. The changes were merged into the following branches:
|
Key changes include:
Removal of PNPM Hook: The
.pnpmfile.cjsfile, which previously converted peer dependencies intodirect dependencies, has been deleted. This workaround is no longer necessary with the adoption of dynamic
imports. The
MODULE.bazelfile has been updated to remove references to it.Dynamic Imports
build_webpackpackage, staticimportstatements forwebpackandwebpack-dev-serverarereplaced with dynamic
import()calls. This defers their loading until they are actively required duringthe process.
ssr-dev-server,browser-syncis now loaded usingcreateRequirerelative to the workspaceroot, improving resolution reliability.
Build Configuration: Associated
BUILD.bazelfiles have been updated to remove dependency declarations that are now obsolete due to the new lazy-loading approach.