Skip to content

Commit

Permalink
Use ambient_srcs instead of global_srcs
Browse files Browse the repository at this point in the history
  • Loading branch information
DavidANeil committed Nov 11, 2020
1 parent 084a69e commit 78eb100
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 44 deletions.
28 changes: 14 additions & 14 deletions internal/js_library/js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ _ATTRS = {
),
"package_name": attr.string(),
"srcs": attr.label_list(allow_files = True),
"global_srcs": attr.label_list(allow_files = True),
"ambient_srcs": attr.label_list(allow_files = True),
}

AmdNamesInfo = provider(
doc = "Non-public API. Provides access to the amd_names attribute of js_library",
fields = {"names": """Mapping from require module names to global variables.
fields = {"names": """Mapping from require module names to ambient variables.
This allows devmode JS sources to load unnamed UMD bundles from third-party libraries."""},
)

def write_amd_names_shim(actions, amd_names_shim, targets):
"""Shim AMD names for UMD bundles that were shipped anonymous.
These are collected from our bootstrap deps (the only place global scripts should appear)
These are collected from our bootstrap deps (the only place ambient scripts should appear)
Args:
actions: starlark rule execution context.actions
Expand All @@ -80,7 +80,7 @@ def write_amd_names_shim(actions, amd_names_shim, targets):
"""

amd_names_shim_content = """// GENERATED by js_library.bzl
// Shim these global symbols which were defined by a bootstrap script
// Shim these ambient symbols which were defined by a bootstrap script
// so that they can be loaded with named require statements.
"""
for t in targets:
Expand All @@ -90,10 +90,10 @@ def write_amd_names_shim(actions, amd_names_shim, targets):
actions.write(amd_names_shim, amd_names_shim_content)

def _impl(ctx):
input_files = ctx.files.srcs + ctx.files.global_srcs + ctx.files.named_module_srcs
input_files = ctx.files.srcs + ctx.files.ambient_srcs + ctx.files.named_module_srcs
all_files = []
typings = []
global_typings = []
ambient_typings = []
js_files = []
named_module_files = []
include_npm_package_info = False
Expand Down Expand Up @@ -130,8 +130,8 @@ def _impl(ctx):
# the tsconfig "lib" attribute
len(file.path.split("/node_modules/")) < 3 and file.path.find("/node_modules/typescript/lib/lib.") == -1
):
if file in ctx.files.global_srcs:
global_typings.append(file)
if file in ctx.files.ambient_srcs:
ambient_typings.append(file)
else:
typings.append(file)

Expand All @@ -148,7 +148,7 @@ def _impl(ctx):
include_npm_package_info = True

# ctx.files.named_module_srcs are merged after ctx.files.srcs
if idx >= len(ctx.files.srcs) + len(ctx.files.global_srcs):
if idx >= len(ctx.files.srcs) + len(ctx.files.ambient_srcs):
named_module_files.append(file)

# every single file on bin should be added here
Expand All @@ -158,14 +158,14 @@ def _impl(ctx):
js_files_depset = depset(js_files)
named_module_files_depset = depset(named_module_files)
typings_depset = depset(typings)
global_typings_depset = depset(global_typings)
ambient_typings_depset = depset(ambient_typings)

files_depsets = [files_depset]
npm_sources_depsets = [files_depset]
direct_sources_depsets = [files_depset]
direct_named_module_sources_depsets = [named_module_files_depset]
typings_depsets = [typings_depset]
global_typings_depsets = [global_typings_depset]
ambient_typings_depsets = [ambient_typings_depset]
js_files_depsets = [js_files_depset]

for dep in ctx.attr.deps:
Expand All @@ -181,8 +181,8 @@ def _impl(ctx):
if DeclarationInfo in dep:
typings_depsets.append(dep[DeclarationInfo].declarations)
direct_sources_depsets.append(dep[DeclarationInfo].declarations)
if getattr(dep[DeclarationInfo], "transitive_global_declarations"):
global_typings_depsets.append(dep[DeclarationInfo].transitive_global_declarations)
if getattr(dep[DeclarationInfo], "transitive_ambient_declarations"):
ambient_typings_depsets.append(dep[DeclarationInfo].transitive_ambient_declarations)
if DefaultInfo in dep:
files_depsets.append(dep[DefaultInfo].files)

Expand Down Expand Up @@ -227,7 +227,7 @@ def _impl(ctx):
providers.append(declaration_info(
declarations = depset(transitive = typings_depsets),
deps = ctx.attr.deps,
global_declarations = depset(transitive = global_typings_depsets)
ambient_declarations = depset(transitive = ambient_typings_depsets)
))

return providers
Expand Down
22 changes: 11 additions & 11 deletions internal/npm_install/generate_build_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const LOCK_FILE_PATH = args[2];
const INCLUDED_FILES = args[3] ? args[3].split(',') : [];
const BAZEL_VERSION = args[4];

const isModuleRegExp = new RegExp('^export', 'm');
const isModuleRegExp = new RegExp('^export|^import', 'm');

if (require.main === module) {
main();
Expand Down Expand Up @@ -877,14 +877,14 @@ function printPackage(pkg: Dep) {
// Files that are part of the npm package not including its nested node_modules
// (filtered by the 'included_files' attribute)
const pkgFiles = includedRunfiles.filter((f: string) => !f.startsWith('node_modules/'));
const globalFiles = pkgFiles.filter((f) => f.endsWith('.d.ts')).filter((f) => {
const ambientFiles = pkgFiles.filter((f) => f.endsWith('.d.ts')).filter((f) => {
const fileContents =
fs.readFileSync(path.join('node_modules', pkg._dir, f), {encoding: 'utf-8'});
return !isModuleRegExp.test(fileContents);
})
const pkgGlobalFilesStarlark = globalFiles.length ? starlarkFiles('srcs', globalFiles) : '';
const nonGlobalFiles = pkgFiles.filter((f) => globalFiles.indexOf(f) === -1);
const pkgFilesStarlark = nonGlobalFiles.length ? starlarkFiles('srcs', nonGlobalFiles) : '';
const pkgAmbientFilesStarlark = ambientFiles.length ? starlarkFiles('srcs', ambientFiles) : '';
const nonAmbientFiles = pkgFiles.filter((f) => ambientFiles.indexOf(f) === -1);
const pkgFilesStarlark = nonAmbientFiles.length ? starlarkFiles('srcs', nonAmbientFiles) : '';

// Files that are in the npm package's nested node_modules
// (filtered by the 'included_files' attribute)
Expand Down Expand Up @@ -916,15 +916,15 @@ function printPackage(pkg: Dep) {
const dtsSources =
filterFiles(pkg._runfiles, [
'.d.ts'
]).filter((f: string) => !f.startsWith('node_modules/') && globalFiles.indexOf(f) === -1);
]).filter((f: string) => !f.startsWith('node_modules/') && ambientFiles.indexOf(f) === -1);
const dtsStarlark =
(dtsSources.length ?
starlarkFiles(
'srcs', dtsSources,
`# ${
pkg._dir} package declaration files (and declaration files in nested node_modules)`) :
'') +
(globalFiles.length ? starlarkFiles('global_srcs', globalFiles) : '');
(ambientFiles.length ? starlarkFiles('ambient_srcs', ambientFiles) : '');

// Flattened list of direct and transitive dependencies hoisted to root by the package manager
const deps = [pkg].concat(pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested));
Expand All @@ -944,7 +944,7 @@ filegroup(
# Files that have side-effects and must always be loaded
filegroup(
name = "${pkg._name}__global_files",${pkgGlobalFilesStarlark}
name = "${pkg._name}__ambient_files",${pkgAmbientFilesStarlark}
)
# Files that are in the npm package's nested node_modules
Expand All @@ -967,15 +967,15 @@ filegroup(
# but not including nested node_modules.
filegroup(
name = "${pkg._name}__all_files",
srcs = [":${pkg._name}__files", ":${pkg._name}__global_files", ":${pkg._name}__not_files"],
srcs = [":${pkg._name}__files", ":${pkg._name}__ambient_files", ":${pkg._name}__not_files"],
)
# The primary target for this package for use in rule deps
js_library(
name = "${pkg._name}",
# direct sources listed for strict deps support
srcs = [":${pkg._name}__files"],
global_srcs = [":${pkg._name}__global_files"],
ambient_srcs = [":${pkg._name}__ambient_files"],
# nested node_modules for this package plus flattened list of direct and transitive dependencies
# hoisted to root by the package manager
deps = [
Expand All @@ -986,7 +986,7 @@ js_library(
# Target is used as dep for main targets to prevent circular dependencies errors
js_library(
name = "${pkg._name}__contents",
srcs = [":${pkg._name}__files", ":${pkg._name}__global_files", ":${
srcs = [":${pkg._name}__files", ":${pkg._name}__ambient_files", ":${
pkg._name}__nested_node_modules"],${namedSourcesStarlark}
visibility = ["//:__subpackages__"],
)
Expand Down
22 changes: 11 additions & 11 deletions internal/npm_install/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const RULE_TYPE = args[1];
const LOCK_FILE_PATH = args[2];
const INCLUDED_FILES = args[3] ? args[3].split(',') : [];
const BAZEL_VERSION = args[4];
const isModuleRegExp = new RegExp('^export', 'm');
const isModuleRegExp = new RegExp('^export|^import', 'm');
if (require.main === module) {
main();
}
Expand Down Expand Up @@ -512,13 +512,13 @@ function printPackage(pkg) {
}
const includedRunfiles = filterFiles(pkg._runfiles, INCLUDED_FILES);
const pkgFiles = includedRunfiles.filter((f) => !f.startsWith('node_modules/'));
const globalFiles = pkgFiles.filter((f) => f.endsWith('.d.ts')).filter((f) => {
const ambientFiles = pkgFiles.filter((f) => f.endsWith('.d.ts')).filter((f) => {
const fileContents = fs.readFileSync(path.join('node_modules', pkg._dir, f), { encoding: 'utf-8' });
return !isModuleRegExp.test(fileContents);
});
const pkgGlobalFilesStarlark = globalFiles.length ? starlarkFiles('srcs', globalFiles) : '';
const nonGlobalFiles = pkgFiles.filter((f) => globalFiles.indexOf(f) === -1);
const pkgFilesStarlark = nonGlobalFiles.length ? starlarkFiles('srcs', nonGlobalFiles) : '';
const pkgAmbientFilesStarlark = ambientFiles.length ? starlarkFiles('srcs', ambientFiles) : '';
const nonAmbientFiles = pkgFiles.filter((f) => ambientFiles.indexOf(f) === -1);
const pkgFilesStarlark = nonAmbientFiles.length ? starlarkFiles('srcs', nonAmbientFiles) : '';
const nestedNodeModules = includedRunfiles.filter((f) => f.startsWith('node_modules/'));
const nestedNodeModulesStarlark = nestedNodeModules.length ? starlarkFiles('srcs', nestedNodeModules) : '';
const notPkgFiles = pkg._files.filter((f) => !f.startsWith('node_modules/') && !includedRunfiles.includes(f));
Expand All @@ -531,11 +531,11 @@ function printPackage(pkg) {
'';
const dtsSources = filterFiles(pkg._runfiles, [
'.d.ts'
]).filter((f) => !f.startsWith('node_modules/') && globalFiles.indexOf(f) === -1);
]).filter((f) => !f.startsWith('node_modules/') && ambientFiles.indexOf(f) === -1);
const dtsStarlark = (dtsSources.length ?
starlarkFiles('srcs', dtsSources, `# ${pkg._dir} package declaration files (and declaration files in nested node_modules)`) :
'') +
(globalFiles.length ? starlarkFiles('global_srcs', globalFiles) : '');
(ambientFiles.length ? starlarkFiles('ambient_srcs', ambientFiles) : '');
const deps = [pkg].concat(pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested));
const depsStarlark = deps.map(dep => `"//${dep._dir}:${dep._name}__contents",`).join('\n ');
let result = `load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
Expand All @@ -551,7 +551,7 @@ filegroup(
# Files that have side-effects and must always be loaded
filegroup(
name = "${pkg._name}__global_files",${pkgGlobalFilesStarlark}
name = "${pkg._name}__ambient_files",${pkgAmbientFilesStarlark}
)
# Files that are in the npm package's nested node_modules
Expand All @@ -574,15 +574,15 @@ filegroup(
# but not including nested node_modules.
filegroup(
name = "${pkg._name}__all_files",
srcs = [":${pkg._name}__files", ":${pkg._name}__global_files", ":${pkg._name}__not_files"],
srcs = [":${pkg._name}__files", ":${pkg._name}__ambient_files", ":${pkg._name}__not_files"],
)
# The primary target for this package for use in rule deps
js_library(
name = "${pkg._name}",
# direct sources listed for strict deps support
srcs = [":${pkg._name}__files"],
global_srcs = [":${pkg._name}__global_files"],
ambient_srcs = [":${pkg._name}__ambient_files"],
# nested node_modules for this package plus flattened list of direct and transitive dependencies
# hoisted to root by the package manager
deps = [
Expand All @@ -593,7 +593,7 @@ js_library(
# Target is used as dep for main targets to prevent circular dependencies errors
js_library(
name = "${pkg._name}__contents",
srcs = [":${pkg._name}__files", ":${pkg._name}__global_files", ":${pkg._name}__nested_node_modules"],${namedSourcesStarlark}
srcs = [":${pkg._name}__files", ":${pkg._name}__ambient_files", ":${pkg._name}__nested_node_modules"],${namedSourcesStarlark}
visibility = ["//:__subpackages__"],
)
Expand Down
16 changes: 8 additions & 8 deletions internal/providers/declaration_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ Note: historically this was a subset of the string-typed "typescript" provider.
"transitive_declarations": """A depset of typings files produced by this rule and all its transitive dependencies.
This prevents needing an aspect in rules that consume the typings, which improves performance.""",
"type_blacklisted_declarations": """A depset of .d.ts files that we should not use to infer JSCompiler types (via tsickle)""",
"global_declarations": "A depset of typings files for global srcs",
"transitive_global_declarations": "A depset of the typings files that should be included to all transitive dependencies",
"ambient_declarations": "A depset of typings files for ambient srcs",
"transitive_ambient_declarations": "A depset of the typings files that should be included to all transitive dependencies",
},
)

def declaration_info(declarations, global_declarations, deps = []):
def declaration_info(declarations, ambient_declarations, deps = []):
"""Constructs a DeclarationInfo including all transitive files needed to type-check from DeclarationInfo providers in a list of deps.
Args:
Expand All @@ -48,18 +48,18 @@ def declaration_info(declarations, global_declarations, deps = []):

# TODO: add some checking actions to ensure the declarations are well-formed and don't have semantic diagnostics
transitive_depsets = [declarations]
transitive_global_depsets = [global_declarations]
transitive_ambient_depsets = [ambient_declarations]
for dep in deps:
if DeclarationInfo in dep:
transitive_depsets.append(dep[DeclarationInfo].transitive_declarations)
if getattr(dep[DeclarationInfo], "transitive_global_declarations", None):
transitive_global_depsets.append(dep[DeclarationInfo].transitive_global_declarations)
if getattr(dep[DeclarationInfo], "transitive_ambient_declarations", None):
transitive_ambient_depsets.append(dep[DeclarationInfo].transitive_ambient_declarations)

return DeclarationInfo(
declarations = declarations,
transitive_declarations = depset(transitive = transitive_depsets),
global_declarations = global_declarations,
transitive_global_declarations = depset(transitive = transitive_global_depsets),
ambient_declarations = ambient_declarations,
transitive_ambient_declarations = depset(transitive = transitive_ambient_depsets),
# Downstream ts_library rules will fail if they don't find this field
# Even though it is only for Google Closure Compiler externs generation
type_blacklisted_declarations = depset(),
Expand Down

0 comments on commit 78eb100

Please sign in to comment.