Skip to content

Commit

Permalink
feat: add generate_local_modules_build_files flag to yarn_install and…
Browse files Browse the repository at this point in the history 
… npm_install rules (#2449)
  • Loading branch information
@mistic
mistic authored Feb 18, 2021
1 parent bc7dc82 commit a6449b7
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 58 deletions.
2 changes: 2 additions & 0 deletions WORKSPACE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ yarn_install(
environment = {
"SOME_USER_ENV": "yarn is great!",
},
generate_local_modules_build_files = False,
included_files = [
"",
".js",
Expand All @@ -226,6 +227,7 @@ npm_install(
environment = {
"SOME_USER_ENV": "npm is cool!",
},
generate_local_modules_build_files = False,
included_files = [
"",
".js",
Expand Down
15 changes: 5 additions & 10 deletions internal/npm_install/generate_build_file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ const WORKSPACE_ROOT_PREFIX = args[4];
const WORKSPACE_ROOT_BASE = WORKSPACE_ROOT_PREFIX ?.split('/')[0];
const STRICT_VISIBILITY = args[5]?.toLowerCase() === 'true';
const INCLUDED_FILES = args[6] ? args[6].split(',') : [];
const BAZEL_VERSION = args[7];
const PACKAGE_PATH = args[8];
const GENERATE_LOCAL_MODULES_BUILD_FILES = (`${args[7]}`.toLowerCase()) === 'true';
const BAZEL_VERSION = args[8];
const PACKAGE_PATH = args[9];

const PUBLIC_VISIBILITY = '//visibility:public';
const LIMITED_VISIBILITY = `@${WORKSPACE}//:__subpackages__`;
Expand Down Expand Up @@ -221,18 +222,12 @@ function generatePackageBuildFiles(pkg: Dep) {
// install a node_module with link:)
const isPkgDirASymlink =
fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink();
// Check if the current package is also written inside the workspace
// NOTE: It's a corner case but fs.realpathSync(.) will not be the root of
// the workspace if symlink_node_modules = False as yarn & npm are run in the root of the
// external repository and anything linked with a relative path would have to be copied
// over via that data attribute
const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`));
// Mark build file as one to symlink instead of generate as the package dir is a symlink, we
// have a BUILD file and the pkg is written inside the workspace
const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace;
const symlinkBuildFile = isPkgDirASymlink && buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES;

// Log if a BUILD file was expected but was not found
if (!symlinkBuildFile && isPkgDirASymlink) {
if (isPkgDirASymlink && !buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES) {
console.log(`[yarn_install/npm_install]: package ${
nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${
fs.realpathSync(nodeModulesPkgDir)}`)
Expand Down
10 changes: 5 additions & 5 deletions internal/npm_install/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ const WORKSPACE_ROOT_PREFIX = args[4];
const WORKSPACE_ROOT_BASE = (_a = WORKSPACE_ROOT_PREFIX) === null || _a === void 0 ? void 0 : _a.split('/')[0];
const STRICT_VISIBILITY = ((_b = args[5]) === null || _b === void 0 ? void 0 : _b.toLowerCase()) === 'true';
const INCLUDED_FILES = args[6] ? args[6].split(',') : [];
const BAZEL_VERSION = args[7];
const PACKAGE_PATH = args[8];
const GENERATE_LOCAL_MODULES_BUILD_FILES = (`${args[7]}`.toLowerCase()) === 'true';
const BAZEL_VERSION = args[8];
const PACKAGE_PATH = args[9];
const PUBLIC_VISIBILITY = '//visibility:public';
const LIMITED_VISIBILITY = `@${WORKSPACE}//:__subpackages__`;
function generateBuildFileHeader(visibility = PUBLIC_VISIBILITY) {
Expand Down Expand Up @@ -123,9 +124,8 @@ function generatePackageBuildFiles(pkg) {
buildFilePath = 'BUILD.bazel';
const nodeModulesPkgDir = `node_modules/${pkg._dir}`;
const isPkgDirASymlink = fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink();
const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`));
const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace;
if (!symlinkBuildFile && isPkgDirASymlink) {
const symlinkBuildFile = isPkgDirASymlink && buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES;
if (isPkgDirASymlink && !buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES) {
console.log(`[yarn_install/npm_install]: package ${nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${fs.realpathSync(nodeModulesPkgDir)}`);
}
let buildFile = printPackage(pkg);
Expand Down
72 changes: 29 additions & 43 deletions internal/npm_install/npm_install.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,29 @@ repository so all files that the package manager depends on must be listed.
doc = """Environment variables to set before calling the package manager.""",
default = {},
),
"generate_local_modules_build_files": attr.bool(
default = True,
doc = """Enables the BUILD files auto generation for local modules installed with `file:` (npm) or `link:` (yarn)
When using a monorepo it's common to have modules that we want to use locally and
publish to an external package repository. This can be achieved using a `js_library` rule
with a `package_name` attribute defined inside the local package `BUILD` file. However,
if the project relies on the local package dependency with `file:` (npm) or `link:` (yarn) to be used outside Bazel, this
could introduce a race condition with both `npm_install` or `yarn_install` rules.
In order to overcome it, a link could be created to the package `BUILD` file from the
npm external Bazel repository (so we can use a local BUILD file instead of an auto generated one),
which require us to set `generate_local_modules_build_files = False` and complete a last step which is writing the
expected targets on that same `BUILD` file to be later used both by `npm_install` or `yarn_install`
rules, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
`<package_name__contents>`, `<package_name__typings>` and the last one just `<package_name>`. If you doubt what those targets
should look like, check the generated `BUILD` file for a given node module.
When true, the rule will follow the default behaviour of auto generating BUILD files for each `node_module` at install time.
When False, the rule will not auto generate BUILD files for `node_modules` that are installed as symlinks for local modules.
""",
),
"included_files": attr.string_list(
doc = """List of file extensions to be included in the npm package targets.
Expand Down Expand Up @@ -123,7 +146,7 @@ data attribute.
),
})

def _create_build_files(repository_ctx, rule_type, node, lock_file):
def _create_build_files(repository_ctx, rule_type, node, lock_file, generate_local_modules_build_files):
repository_ctx.report_progress("Processing node_modules: installing Bazel packages and generating BUILD files")
if repository_ctx.attr.manual_build_file_contents:
repository_ctx.file("manual_build_file_contents", repository_ctx.attr.manual_build_file_contents)
Expand All @@ -137,6 +160,7 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file):
_workspace_root_prefix(repository_ctx),
str(repository_ctx.attr.strict_visibility),
",".join(repository_ctx.attr.included_files),
str(generate_local_modules_build_files),
native.bazel_version,
repository_ctx.attr.package_path,
# double the default timeout in case of many packages, see #2231
Expand Down Expand Up @@ -344,7 +368,7 @@ cd /D "{root}" && "{npm}" {npm_args}

_symlink_node_modules(repository_ctx)

_create_build_files(repository_ctx, "npm_install", node, repository_ctx.attr.package_lock_json)
_create_build_files(repository_ctx, "npm_install", node, repository_ctx.attr.package_lock_json, repository_ctx.attr.generate_local_modules_build_files)

npm_install = repository_rule(
attrs = dict(COMMON_ATTRIBUTES, **{
Expand Down Expand Up @@ -374,26 +398,7 @@ See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of su
This rule will set the environment variable `BAZEL_NPM_INSTALL` to '1' (unless it
set to another value in the environment attribute). Scripts may use to this to
check if yarn is being run by the `npm_install` repository rule.
**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL**
When using a monorepo it's common to have modules that we want to use locally and
publish to an external package repository. This can be achieved using a `js_library` rule
with a `package_name` attribute defined inside the local package `BUILD` file. However,
if the project relies on the local package dependency with `file:`, this could introduce a
race condition with the `npm_install` rule.
In order to overcome it, a link will be created to the package `BUILD` file from the
npm external Bazel repository, which require us to complete a last step which is writing
the expected targets on that same `BUILD` file to be later used by the `npm_install`
rule, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
`<package_name__contents>`, `<package_name__typings>` and the last
one just `<package_name>`.
If you doubt what those targets should look like, check the
generated `BUILD` file for a given node module.""",
check if yarn is being run by the `npm_install` repository rule.""",
implementation = _npm_install_impl,
)

Expand Down Expand Up @@ -499,7 +504,7 @@ cd /D "{root}" && "{yarn}" {yarn_args}

_symlink_node_modules(repository_ctx)

_create_build_files(repository_ctx, "yarn_install", node, repository_ctx.attr.yarn_lock)
_create_build_files(repository_ctx, "yarn_install", node, repository_ctx.attr.yarn_lock, repository_ctx.attr.generate_local_modules_build_files)

yarn_install = repository_rule(
attrs = dict(COMMON_ATTRIBUTES, **{
Expand Down Expand Up @@ -550,25 +555,6 @@ to yarn so that the local cache is contained within the external repository.
This rule will set the environment variable `BAZEL_YARN_INSTALL` to '1' (unless it
set to another value in the environment attribute). Scripts may use to this to
check if yarn is being run by the `yarn_install` repository rule.
**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL**
When using a monorepo it's common to have modules that we want to use locally and
publish to an external package repository. This can be achieved using a `js_library` rule
with a `package_name` attribute defined inside the local package `BUILD` file. However,
if the project relies on the local package dependency with `link:`, this could introduce a
race condition with the `yarn_install` rule.
In order to overcome it, a link will be created to the package `BUILD` file from the
npm external Bazel repository, which require us to complete a last step which is writing
the expected targets on that same `BUILD` file to be later used by the `yarn_install`
rule, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
`<package_name__contents>`, `<package_name__typings>` and the last
one just `<package_name>`.
If you doubt what those targets should look like, check the
generated `BUILD` file for a given node module.""",
check if yarn is being run by the `yarn_install` repository rule.""",
implementation = _yarn_install_impl,
)

0 comments on commit a6449b7

Please sign in to comment.