Skip to content

Commit

Permalink
refactor: pkg_npm attributes renames packages=>nested_packages & repl…
Browse files Browse the repository at this point in the history
…acements=>substitutions

BREAKING CHANGE:
* pkg_npm attribute packages renamed to nested_packages
* pkg_npm attribute replacements renamed to substitutions
  • Loading branch information
gregmagolan authored and alexeagle committed Dec 19, 2019
1 parent 6d731cf commit 7e1b7df
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 34 deletions.
2 changes: 1 addition & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pkg_npm(
# Don't replace the default 0.0.0-PLACEHOLDER for this pkg_npm since
# we are packaging up the packager itself and this replacement will break it
replace_with_version = "",
replacements = COMMON_REPLACEMENTS,
substitutions = COMMON_REPLACEMENTS,
deps = [
"//internal:package_contents",
"//internal/bazel_integration_test:package_contents",
Expand Down
2 changes: 1 addition & 1 deletion e2e/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ e2e_integration_test(
"//packages/typescript:npm_package": "@bazel/typescript",
},
# use these package.json packages instead
package_json_replacements = {
package_json_substitutions = {
"typescript": tsc_version,
},
workspace_root = "typescript",
Expand Down
8 changes: 4 additions & 4 deletions internal/bazel_integration_test/bazel_integration_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ module.exports = {{
bazelrcImports: {{ {TMPL_bazelrc_imports} }},
npmPackages: {{ {TMPL_npm_packages} }},
checkNpmPackages: [ {TMPL_check_npm_packages} ],
packageJsonRepacements: {{ {TMPL_package_json_replacements} }},
packageJsonRepacements: {{ {TMPL_package_json_substitutions} }},
}};
""".format(
TMPL_workspace_root = ctx.files.workspace_files[0].dirname,
Expand All @@ -63,7 +63,7 @@ module.exports = {{
TMPL_bazelrc_imports = ", ".join(["'%s': '%s'" % (ctx.attr.bazelrc_imports[f], _to_manifest_path(ctx, f.files.to_list()[0])) for f in ctx.attr.bazelrc_imports]),
TMPL_npm_packages = ", ".join(["'%s': '%s'" % (ctx.attr.npm_packages[f], _to_manifest_path(ctx, f.files.to_list()[0])) for f in ctx.attr.npm_packages]),
TMPL_check_npm_packages = ", ".join(["'%s'" % s for s in ctx.attr.check_npm_packages]),
TMPL_package_json_replacements = ", ".join(["'%s': '%s'" % (f, ctx.attr.package_json_replacements[f]) for f in ctx.attr.package_json_replacements]),
TMPL_package_json_substitutions = ", ".join(["'%s': '%s'" % (f, ctx.attr.package_json_substitutions[f]) for f in ctx.attr.package_json_substitutions]),
),
)

Expand Down Expand Up @@ -194,15 +194,15 @@ npm_packages = {
}
```""",
),
"package_json_replacements": attr.string_dict(
"package_json_substitutions": attr.string_dict(
doc = """A string dictionary of other package.json package replacements to make.
This can be used for integration testing against multiple external npm dependencies without duplicating code. For example,
```
[bazel_integration_test(
name = "e2e_typescript_%s" % tsc_version.replace(".", "_"),
...
package_json_replacements = {
package_json_substitutions = {
"typescript": tsc_version,
},
...
Expand Down
24 changes: 12 additions & 12 deletions internal/pkg_npm/packager.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ function mkdirp(p) {
}
}

function copyWithReplace(src, dest, replacements, renameBuildFiles) {
function copyWithReplace(src, dest, substitutions, renameBuildFiles) {
mkdirp(path.dirname(dest));
if (!isBinary(src)) {
let content = fs.readFileSync(src, {encoding: 'utf-8'});
replacements.forEach(r => {
substitutions.forEach(r => {
const [regexp, newvalue] = r;
content = content.replace(regexp, newvalue);
});
Expand All @@ -59,20 +59,20 @@ function unquoteArgs(s) {
function main(args) {
args = fs.readFileSync(args[0], {encoding: 'utf-8'}).split('\n').map(unquoteArgs);
const
[outDir, baseDir, srcsArg, binDir, genDir, depsArg, packagesArg, replacementsArg, packPath,
[outDir, baseDir, srcsArg, binDir, genDir, depsArg, packagesArg, substitutionsArg, packPath,
publishPath, replaceWithVersion, stampFile, vendorExternalArg, renameBuildFilesArg,
runNpmTemplatePath] = args;
const renameBuildFiles = parseInt(renameBuildFilesArg);

const replacements = [
const substitutions = [
// Strip content between BEGIN-INTERNAL / END-INTERNAL comments
[/(#|\/\/)\s+BEGIN-INTERNAL[\w\W]+?END-INTERNAL/g, ''],
];
const rawReplacements = JSON.parse(replacementsArg);
const rawReplacements = JSON.parse(substitutionsArg);
for (let key of Object.keys(rawReplacements)) {
replacements.push([new RegExp(key, 'g'), rawReplacements[key]])
substitutions.push([new RegExp(key, 'g'), rawReplacements[key]])
}
// Replace version last so that earlier replacements can add
// Replace version last so that earlier substitutions can add
// the version placeholder
if (replaceWithVersion) {
let version = '0.0.0';
Expand All @@ -92,7 +92,7 @@ function main(args) {
version = versionTag.split(' ')[1].trim();
}
}
replacements.push([new RegExp(replaceWithVersion, 'g'), version]);
substitutions.push([new RegExp(replaceWithVersion, 'g'), version]);
}

// src like baseDir/my/path is just copied to outDir/my/path
Expand All @@ -101,7 +101,7 @@ function main(args) {
if (src.startsWith('external/')) {
// If srcs is from external workspace drop the external/wksp portion
copyWithReplace(
src, path.join(outDir, src.split('/').slice(2).join('/')), replacements,
src, path.join(outDir, src.split('/').slice(2).join('/')), substitutions,
renameBuildFiles);
} else {
// Source is from local workspace
Expand All @@ -111,7 +111,7 @@ function main(args) {
`generated file should belong in 'deps' instead.`);
}
copyWithReplace(
src, path.join(outDir, path.relative(baseDir, src)), replacements, renameBuildFiles);
src, path.join(outDir, path.relative(baseDir, src)), substitutions, renameBuildFiles);
}
}

Expand Down Expand Up @@ -140,7 +140,7 @@ function main(args) {
// Deps like bazel-bin/baseDir/my/path is copied to outDir/my/path.
for (dep of depsArg.split(',').filter(s => !!s)) {
try {
copyWithReplace(dep, outPath(dep), replacements, renameBuildFiles);
copyWithReplace(dep, outPath(dep), substitutions, renameBuildFiles);
} catch (e) {
console.error(`Failed to copy ${dep} to ${outPath(dep)}`);
throw e;
Expand Down Expand Up @@ -168,7 +168,7 @@ function main(args) {
return file;
}
copyWithReplace(
path.join(base, file), path.join(outDir, outFile()), replacements, renameBuildFiles);
path.join(base, file), path.join(outDir, outFile()), substitutions, renameBuildFiles);
}
}
fs.readdirSync(pkg).forEach(f => {
Expand Down
16 changes: 8 additions & 8 deletions internal/pkg_npm/pkg_npm.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def create_package(ctx, deps_sources, nested_packages):
args.add(ctx.genfiles_dir.path)
args.add_joined(filtered_deps_sources, join_with = ",", omit_if_empty = False)
args.add_joined([p.path for p in nested_packages], join_with = ",", omit_if_empty = False)
args.add(ctx.attr.replacements)
args.add(ctx.attr.substitutions)
args.add_all([ctx.outputs.pack.path, ctx.outputs.publish.path])
args.add(ctx.attr.replace_with_version)
args.add(ctx.version_file.path if stamp else "")
Expand Down Expand Up @@ -111,7 +111,7 @@ def _pkg_npm(ctx):
sources = depset(transitive = sources_depsets)

# Note: to_list() should be called once per rule!
package_dir = create_package(ctx, sources.to_list(), ctx.files.packages)
package_dir = create_package(ctx, sources.to_list(), ctx.files.nested_packages)

return [DefaultInfo(
files = depset([package_dir]),
Expand All @@ -123,15 +123,15 @@ PKG_NPM_ATTRS = {
doc = """Files inside this directory which are simply copied into the package.""",
allow_files = True,
),
"nested_packages": attr.label_list(
doc = """Other pkg_npm rules whose content is copied into this package.""",
allow_files = True,
),
"node_context_data": attr.label(
default = "@build_bazel_rules_nodejs//internal:node_context_data",
providers = [NodeContextInfo],
doc = "Internal use only",
),
"packages": attr.label_list(
doc = """Other pkg_npm rules whose content is copied into this package.""",
allow_files = True,
),
"rename_build_files": attr.bool(
doc = """If set BUILD and BUILD.bazel files are prefixed with `_` in the npm package.
The default is True since npm packages that contain BUILD files don't work with
Expand All @@ -143,7 +143,7 @@ PKG_NPM_ATTRS = {
See the section on stamping in the README.""",
default = "0.0.0-PLACEHOLDER",
),
"replacements": attr.string_dict(
"substitutions": attr.string_dict(
doc = """Key-value pairs which are replaced in all the files while building the package.""",
),
"vendor_external": attr.string_list(
Expand Down Expand Up @@ -184,7 +184,7 @@ pkg_npm(
name = "my_package",
srcs = ["package.json"],
deps = [":my_typescript_lib"],
replacements = {"//internal/": "//"},
substitutions = {"//internal/": "//"},
)
```
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg_npm/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ pkg_npm(
"some_file",
"@internal_npm_package_test_vendored_external//:vendored_external_file",
],
nested_packages = [":dependent_pkg"],
node_context_data = ":force_stamp",
packages = [":dependent_pkg"],
replacements = {"replace_me": "replaced"},
substitutions = {"replace_me": "replaced"},
vendor_external = [
"internal_npm_package_test_vendored_external",
],
Expand Down
4 changes: 2 additions & 2 deletions packages/typescript/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ pkg_npm(
"@npm_bazel_typescript//:package_contents",
],
build_file_content = "",
packages = [
nested_packages = [
"@build_bazel_rules_typescript//:npm_bazel_typescript_package",
],
replacements = TYPESCRIPT_REPLACEMENTS,
substitutions = TYPESCRIPT_REPLACEMENTS,
vendor_external = [
"npm_bazel_typescript",
"build_bazel_rules_typescript",
Expand Down
2 changes: 1 addition & 1 deletion packages/worker/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pkg_npm(
"README.md",
"package.json",
],
replacements = {
substitutions = {
# Fix the require() statement that loads the worker_protocol.proto file
# we are re-rooting the sources into the @bazel/worker package so it's no longer
# relative to the build_bazel_rules_typescript workspace.
Expand Down
6 changes: 3 additions & 3 deletions tools/defaults.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ def pkg_npm(**kwargs):
"//examples:__pkg__",
])

# Default replacements to scrub things like skylib references
replacements = kwargs.pop("replacements", _COMMON_REPLACEMENTS)
# Default substitutions to scrub things like skylib references
substitutions = kwargs.pop("substitutions", _COMMON_REPLACEMENTS)

# Finally call through to the rule with our defaults set
_pkg_npm(
deps = deps,
replacements = replacements,
substitutions = substitutions,
visibility = visibility,
**kwargs
)
Expand Down

0 comments on commit 7e1b7df

Please sign in to comment.