Skip to content
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

feat: add generate_local_modules_build_files flag to yarn_install and npm_install rules #2449

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions WORKSPACE
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
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
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
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
Copy link

Choose a reason for hiding this comment

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

npm also supports link: and yarn also supports file:, so i'm not sure why these parentheticals are here?

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,
)