feat(typescript): add resolve_json_module support to ts_project #2384
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As of TypeScript v2.9, `tsc` can read and extract types from `.json` files by using the `resolveJsonModule` option. https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-9.html#new---resolvejsonmodule The new behavior allows people to pass `resolve_json_module` to `ts_project` in order to expect `.json` files from `tsc`. This change was modeled after the previous `allow_js` work in bazel-contrib#2260. Fixes bazel-contrib#2365
lencioni
requested review from
alexeagle,
gregmagolan,
mattem and
soldair
as code owners
This was referenced Jan 6, 2021
lencioni
commented
Jan 11, 2021
Comment on lines
+699
to
+703
| js_outs.extend(_out_paths(srcs, out_dir, root_dir, False, resolve_json_module, ".js")) | ||
| if source_map and not emit_declaration_only: | ||
| map_outs.extend(_out_paths(srcs, out_dir, root_dir, False, ".js.map")) | ||
| map_outs.extend(_out_paths(srcs, out_dir, root_dir, False, False, ".js.map")) | ||
| if declaration or composite: | ||
| typings_outs.extend(_out_paths(srcs, typings_out_dir, root_dir, allow_js, ".d.ts")) | ||
| typings_outs.extend(_out_paths(srcs, typings_out_dir, root_dir, allow_js, False, ".d.ts")) |
There was a problem hiding this comment.
I think this is maybe not quite right. We need the json files even if emit_declaration_only is true. I think it might make more sense to put these in typings_outs?
|
I've been trying this out in my repo (which is currently using rules_nodejs 2.3.1) by using patch-package. Here's my patch: diff --git a/node_modules/@bazel/typescript/internal/ts_project.bzl b/node_modules/@bazel/typescript/internal/ts_project.bzl
index 080817a..86ab101 100755
--- a/node_modules/@bazel/typescript/internal/ts_project.bzl
+++ b/node_modules/@bazel/typescript/internal/ts_project.bzl
@@ -76,8 +76,9 @@ def _calculate_root_dir(ctx):
# It's a non-breaking change to relax this constraint later, but would be
# a breaking change to restrict it further.
allow_js = True
+ resolve_json_module = True
for src in ctx.files.srcs:
- if _is_ts_src(src.path, allow_js):
+ if _is_ts_src(src.path, allow_js, resolve_json_module):
if src.is_source:
some_source_path = src.path
else:
@@ -260,6 +261,7 @@ def _validate_options_impl(ctx):
declaration_map = ctx.attr.declaration_map,
composite = ctx.attr.composite,
emit_declaration_only = ctx.attr.emit_declaration_only,
+ resolve_json_module = ctx.attr.resolve_json_module,
source_map = ctx.attr.source_map,
incremental = ctx.attr.incremental,
ts_build_info_file = ctx.attr.ts_build_info_file,
@@ -292,6 +294,7 @@ validate_options = rule(
"emit_declaration_only": attr.bool(),
"extends": attr.label_list(allow_files = [".json"]),
"incremental": attr.bool(),
+ "resolve_json_module": attr.bool(),
"source_map": attr.bool(),
"target": attr.string(),
"ts_build_info_file": attr.string(),
@@ -300,19 +303,34 @@ validate_options = rule(
},
)
-def _is_ts_src(src, allow_js):
+def _is_ts_src(src, allow_js, resolve_json_module):
if not src.endswith(".d.ts") and (src.endswith(".ts") or src.endswith(".tsx")):
return True
+
+ if resolve_json_module and src.endswith(".json"):
+ return True
+
return allow_js and (src.endswith(".js") or src.endswith(".jsx"))
-def _out_paths(srcs, outdir, rootdir, allow_js, ext):
+def _out_paths(srcs, outdir, rootdir, allow_js, resolve_json_module, ext):
rootdir_replace_pattern = rootdir + "/" if rootdir else ""
- return [
+
+ out_paths = [
_join(outdir, f[:f.rindex(".")].replace(rootdir_replace_pattern, "") + ext)
for f in srcs
- if _is_ts_src(f, allow_js)
+ if _is_ts_src(f, allow_js, False)
]
+ if resolve_json_module == True:
+ out_paths = out_paths + [
+ _join(outdir, f.replace(rootdir_replace_pattern, ""))
+ for f in srcs
+ if f.endswith(".json")
+ ]
+
+ return out_paths
+
+
def ts_project_macro(
name = "tsconfig",
tsconfig = None,
@@ -321,6 +339,7 @@ def ts_project_macro(
deps = [],
extends = None,
allow_js = False,
+ resolve_json_module = False,
declaration = False,
source_map = False,
declaration_map = False,
@@ -561,10 +580,16 @@ def ts_project_macro(
"""
if srcs == None:
+ globs = ["**/*.ts", "**/*.tsx"]
+
if allow_js == True:
- srcs = native.glob(["**/*.ts", "**/*.tsx", "**/*.js", "**/*.jsx"])
- else:
- srcs = native.glob(["**/*.ts", "**/*.tsx"])
+ globs = globs + ["**/*.js", "**/*.jsx"]
+
+ if resolve_json_module == True:
+ globs = globs + ["**/*.json"]
+
+ srcs = native.glob(globs)
+
extra_deps = []
if type(tsconfig) == type(dict()):
@@ -580,6 +605,7 @@ def ts_project_macro(
declaration_map = compiler_options.setdefault("declarationMap", declaration_map)
emit_declaration_only = compiler_options.setdefault("emitDeclarationOnly", emit_declaration_only)
allow_js = compiler_options.setdefault("allowJs", allow_js)
+ resolve_json_module = compiler_options.setdefault("resolveJsonModule", resolve_json_module)
# These options are always passed on the tsc command line so don't include them
# in the tsconfig. At best they're redundant, but at worst we'll have a conflict
@@ -594,7 +620,7 @@ def ts_project_macro(
write_tsconfig(
name = "_gen_tsconfig_%s" % name,
config = tsconfig,
- files = srcs,
+ files = [s for s in srcs if _is_ts_src(s, allow_js, resolve_json_module)],
extends = Label("//%s:%s" % (native.package_name(), name)).relative(extends) if extends else None,
out = "tsconfig_%s.json" % name,
)
@@ -619,6 +645,7 @@ def ts_project_macro(
ts_build_info_file = ts_build_info_file,
emit_declaration_only = emit_declaration_only,
allow_js = allow_js,
+ resolve_json_module = resolve_json_module,
tsconfig = tsconfig,
extends = extends,
)
@@ -671,10 +698,10 @@ def ts_project_macro(
declaration_dir = declaration_dir,
out_dir = out_dir,
root_dir = root_dir,
- js_outs = _out_paths(srcs, out_dir, root_dir, False, ".js") if not emit_declaration_only else [],
- map_outs = _out_paths(srcs, out_dir, root_dir, False, ".js.map") if source_map and not emit_declaration_only else [],
- typings_outs = _out_paths(srcs, typings_out_dir, root_dir, allow_js, ".d.ts") if declaration or composite else [],
- typing_maps_outs = _out_paths(srcs, typings_out_dir, root_dir, allow_js, ".d.ts.map") if declaration_map else [],
+ js_outs = _out_paths(srcs, out_dir, root_dir, False, resolve_json_module, ".js") if not emit_declaration_only else [],
+ map_outs = _out_paths(srcs, out_dir, root_dir, False, False, ".js.map") if source_map and not emit_declaration_only else [],
+ typings_outs = _out_paths(srcs, typings_out_dir, root_dir, allow_js, False, ".d.ts") if declaration or composite else [],
+ typing_maps_outs = _out_paths(srcs, typings_out_dir, root_dir, allow_js, False, ".d.ts.map") if declaration_map else [],
buildinfo_out = tsbuildinfo_path if composite or incremental else None,
tsc = tsc,
link_workspace_root = link_workspace_root,
diff --git a/node_modules/@bazel/typescript/internal/ts_project_options_validator.js b/node_modules/@bazel/typescript/internal/ts_project_options_validator.js
index ba63838..2fd41c7 100755
--- a/node_modules/@bazel/typescript/internal/ts_project_options_validator.js
+++ b/node_modules/@bazel/typescript/internal/ts_project_options_validator.js
@@ -63,6 +63,7 @@ function main(_a) {
}
}
check('allowJs', 'allow_js');
+ check('resolveJsonModule', 'resolve_json_module');
check('declarationMap', 'declaration_map');
check('emitDeclarationOnly', 'emit_declaration_only');
check('sourceMap', 'source_map');
@@ -81,7 +82,7 @@ function main(_a) {
}
// We have to write an output so that Bazel needs to execute this action.
// Make the output change whenever the attributes changed.
- require('fs').writeFileSync(output, "\n// " + process.argv[1] + " checked attributes for " + target + "\n// allow_js: " + attrs.allow_js + "\n// composite: " + attrs.composite + "\n// declaration: " + attrs.declaration + "\n// declaration_map: " + attrs.declaration_map + "\n// incremental: " + attrs.incremental + "\n// source_map: " + attrs.source_map + "\n// emit_declaration_only: " + attrs.emit_declaration_only + "\n// ts_build_info_file: " + attrs.ts_build_info_file + "\n", 'utf-8');
+ require('fs').writeFileSync(output, "\n// " + process.argv[1] + " checked attributes for " + target + "\n// allow_js: " + attrs.allow_js + "\n//resolve_json_module: " + attrs.resolve_json_module + "\n// composite: " + attrs.composite + "\n// declaration: " + attrs.declaration + "\n// declaration_map: " + attrs.declaration_map + "\n// incremental: " + attrs.incremental + "\n// source_map: " + attrs.source_map + "\n// emit_declaration_only: " + attrs.emit_declaration_only + "\n// ts_build_info_file: " + attrs.ts_build_info_file + "\n", 'utf-8');
return 0;
}
if (require.main === module) {Unfortunately, it doesn't seem to work and I don't really know what to try next. One issue I ran into with this is when I included the json files in outputs, Bazel would give me errors about some .json files being used as both an input and as an output. Since tsc isn't making any changes to the .json files, I think that maybe we don't need to include them in outputs? So I modified my patch to be basically the same except just not include the json files in the outputs, and I end up with problems where .json files can't be found in other projects, e.g.: I can make this better by including all .json files in the Any suggestions of what to try next? |
|
I've been digging into this more, and I believe that this is not very workable as a full solution at this time. I'll post more information about this on the parent issue. |
|
For posterity, here's my WIP diff based on v2.3.1 diff --git a/bazel/ts_project.bzl b/bazel/ts_project.bzl
index cbc78db2a3f..5c68d877066 100644
--- a/bazel/ts_project.bzl
+++ b/bazel/ts_project.bzl
@@ -52,6 +52,7 @@ _ATTRS = {
# that compiler might allow more sources than tsc does.
"srcs": attr.label_list(allow_files = True, mandatory = True),
"supports_workers": attr.bool(default = False),
+ "resolve_json_module": attr.bool(default = False),
"tsc": attr.label(default = Label(_DEFAULT_TSC), executable = True, cfg = "target"),
"tsconfig": attr.label(mandatory = True, allow_single_file = [".json"]),
}
@@ -85,8 +86,9 @@ def _calculate_root_dir(ctx):
# It's a non-breaking change to relax this constraint later, but would be
# a breaking change to restrict it further.
allow_js = True
+ resolve_json_module = True
for src in ctx.files.srcs:
- if _is_ts_src(src.path, allow_js):
+ if _is_ts_src(src.path, allow_js, resolve_json_module):
if src.is_source:
some_source_path = src.path
else:
@@ -200,8 +202,30 @@ def _ts_project_impl(ctx):
ctx.outputs.buildinfo_out.path,
])
outputs.append(ctx.outputs.buildinfo_out)
+
runtime_outputs = json_outs + ctx.outputs.js_outs + ctx.outputs.map_outs
+
typings_outputs = ctx.outputs.typings_outs + ctx.outputs.typing_maps_outs + [s for s in ctx.files.srcs if s.path.endswith(".d.ts")]
+
+ #json_srcs = [s for s in ctx.files.srcs if s.path.endswith(".json")]
+ json_srcs = []
+ if ctx.attr.resolve_json_module == True:
+ for src in ctx.files.srcs:
+ if src.path.endswith(".json"):
+ declared = ctx.actions.declare_file(src.short_path[len(ctx.label.package) + 1:])
+ ctx.actions.expand_template(
+ output = declared,
+ substitutions = {},
+ template = src,
+ )
+ json_srcs.append(declared)
+
+ # resolve_json_module is True, which means that TypeScript will read
+ # and extract types from .json files, but it will not emit declaration
+ # files for these. We need to ensure that these .json files are passed
+ # along in the DeclarationInfo provider.
+ typings_outputs = typings_outputs + json_srcs
+
default_outputs_depset = depset(runtime_outputs) if len(runtime_outputs) else depset(typings_outputs)
if len(outputs) > 0:
@@ -269,6 +293,7 @@ def _validate_options_impl(ctx):
declaration_map = ctx.attr.declaration_map,
composite = ctx.attr.composite,
emit_declaration_only = ctx.attr.emit_declaration_only,
+ resolve_json_module = ctx.attr.resolve_json_module,
source_map = ctx.attr.source_map,
incremental = ctx.attr.incremental,
ts_build_info_file = ctx.attr.ts_build_info_file,
@@ -301,6 +326,7 @@ validate_options = rule(
"emit_declaration_only": attr.bool(),
"extends": attr.label_list(allow_files = [".json"]),
"incremental": attr.bool(),
+ "resolve_json_module": attr.bool(),
"source_map": attr.bool(),
"target": attr.string(),
"ts_build_info_file": attr.string(),
@@ -309,17 +335,22 @@ validate_options = rule(
},
)
-def _is_ts_src(src, allow_js):
+def _is_ts_src(src, allow_js, resolve_json_module):
if not src.endswith(".d.ts") and (src.endswith(".ts") or src.endswith(".tsx")):
return True
+
+ if resolve_json_module and src.endswith(".json"):
+ return True
+
return allow_js and (src.endswith(".js") or src.endswith(".jsx"))
def _out_paths(srcs, outdir, rootdir, allow_js, ext):
rootdir_replace_pattern = rootdir + "/" if rootdir else ""
+
return [
_join(outdir, f[:f.rindex(".")].replace(rootdir_replace_pattern, "") + ext)
for f in srcs
- if _is_ts_src(f, allow_js)
+ if _is_ts_src(f, allow_js, False)
]
def ts_project_macro(
@@ -330,6 +361,7 @@ def ts_project_macro(
deps = [],
extends = None,
allow_js = False,
+ resolve_json_module = False,
declaration = False,
source_map = False,
declaration_map = False,
@@ -570,10 +602,16 @@ def ts_project_macro(
"""
if srcs == None:
+ globs = ["**/*.ts", "**/*.tsx"]
+
if allow_js == True:
- srcs = native.glob(["**/*.ts", "**/*.tsx", "**/*.js", "**/*.jsx"])
- else:
- srcs = native.glob(["**/*.ts", "**/*.tsx"])
+ globs = globs + ["**/*.js", "**/*.jsx"]
+
+ if resolve_json_module == True:
+ globs = globs + ["**/*.json"]
+
+ srcs = native.glob(globs)
+
extra_deps = []
if type(tsconfig) == type(dict()):
@@ -589,6 +627,7 @@ def ts_project_macro(
declaration_map = compiler_options.setdefault("declarationMap", declaration_map)
emit_declaration_only = compiler_options.setdefault("emitDeclarationOnly", emit_declaration_only)
allow_js = compiler_options.setdefault("allowJs", allow_js)
+ resolve_json_module = compiler_options.setdefault("resolveJsonModule", resolve_json_module)
# These options are always passed on the tsc command line so don't include them
# in the tsconfig. At best they're redundant, but at worst we'll have a conflict
@@ -603,7 +642,7 @@ def ts_project_macro(
write_tsconfig(
name = "_gen_tsconfig_%s" % name,
config = tsconfig,
- files = srcs,
+ files = [s for s in srcs if _is_ts_src(s, allow_js, resolve_json_module)],
extends = Label("//%s:%s" % (native.package_name(), name)).relative(extends) if extends else None,
out = "tsconfig_%s.json" % name,
)
@@ -628,6 +667,7 @@ def ts_project_macro(
ts_build_info_file = ts_build_info_file,
emit_declaration_only = emit_declaration_only,
allow_js = allow_js,
+ resolve_json_module = resolve_json_module,
tsconfig = tsconfig,
extends = extends,
)
@@ -687,6 +727,7 @@ def ts_project_macro(
typings_outs = _out_paths(srcs, typings_out_dir, root_dir, allow_js, ".d.ts") if declaration or composite else [],
typing_maps_outs = _out_paths(srcs, typings_out_dir, root_dir, allow_js, ".d.ts.map") if declaration_map else [],
buildinfo_out = tsbuildinfo_path if composite or incremental else None,
+ resolve_json_module = resolve_json_module,
tsc = tsc,
link_workspace_root = link_workspace_root,
supports_workers = select({ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2365
What is the new behavior?
As of TypeScript v2.9,
tsccan read and extract types from.jsonfiles by using the
resolveJsonModuleoption.https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-9.html#new---resolvejsonmodule
The new behavior allows people to pass
resolve_json_moduletots_projectin order to expect.jsonfiles fromtsc.This change was modeled after the previous
allow_jswork in #2260.Fixes #2365
Does this PR introduce a breaking change?
Other information
I don't really know what I'm doing here, so any advice on how to improve this PR is greatly appreciated.