-
-
Notifications
You must be signed in to change notification settings - Fork 149
Add replace_package module extension tag #2289
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
base: main
Are you sure you want to change the base?
Add replace_package module extension tag #2289
Conversation
|
|
||
console.log('[utils_module] All tests passed ✓'); | ||
|
||
export function testUtilsModule() { |
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.
Delete?
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.
Deleted
f1bc621
to
ee5b4e4
Compare
bazel_dep(name = "platforms", version = "0.0.5") | ||
bazel_dep(name = "aspect_rules_js", version = "0.0.0") | ||
bazel_dep(name = "aspect_bazel_lib", version = "2.7.7") | ||
bazel_dep(name = "bazel_skylib", version = "1.5.0") |
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.
Is this used?
path = "../lodash_module", | ||
) | ||
|
||
bazel_dep(name = "platforms", version = "0.0.5") |
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.
Is this used?
If you need to replace a package in a non-root module: | ||
1. Move the npm_replace_package() call to your root MODULE.bazel file | ||
2. Alternatively, use the replace_packages attribute in npm_translate_lock() \ |
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.
Probably delete this suggestion since it is deprecated?
|
||
# Validate that only root modules (or isolated extensions) use npm_replace_package | ||
for mod in module_ctx.modules: | ||
_fail_on_non_root_overrides(module_ctx, mod, "npm_replace_package") |
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.
Isn't it easier to put this in the loop below where we're already looping for attr in mod.tags.npm_replace_package:
?
attrs.pop("exclude_package_contents") # Use tag classes only for MODULE.bazel | ||
attrs.pop("exclude_package_contents") # Use npm_exclude_package_contents tag instead | ||
|
||
# TODO(3.0): remove replace_packages attribute in favor of npm_replace_package tag |
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 would we do in 3.0? If the replace_packages
attribute is removed I'd think there would be nothing TODO here...?
Changes are visible to end-users: yes
feat: moved replace package to a tag class for bzlmod
Test plan