Migrate to ts_project()
#50
Comments
This was referenced Jul 4, 2022
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. This is just a `ts_project()` with some better defaults to reduce boilerplate. Also updates the `tsconfig.json` to set up `rootDirs` correctly and remove the WebDriverIO type which apparently isn't necessary.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. `exec` is generally preferred, however it is not easily supported by `ts_project()` because we need to explicitly list all the Bazel artifact roots in `rootDirs` (https://bazelbuild.github.io/rules_nodejs/TypeScript.html#ts_project). `exec` dynamically computes artifact roots based on the target's configuration, which can change arbitrarily based on build flags and how targets are used. It's not fit to be hard-coded in a `tsconfig.json`. I left a `TODO` to switch back to `exec` after updating to `rules_js`, since it should be able to fix this particular rough edge.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. `ts_project()` doesn't return this provider, so it can't be required.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
… use `ts_project()`. Refs #50. This migrates all the core infrastructure to `ts_project()` except for examples and the `prerender_component()` macro. Mostly it was a drop in replacement with a couple exceptions: 1. `import 'webdriverio';` doesn't work because it isn't actually an ES module. Even if we get TypeScript to pass here, it fails at runtime. The only way I could get a global import that was elided at runtime was to use project references, not sure if there's a better way of doing this. 2. `prerender_component_publish_files()` script file names have changed because `ts_project()` AFAICT does not have an ES5/ES6 distinction, it just uses whatever the associated `ts_project()` tells it to. This means `*.externs.js` files are gone, and `*.mjs` files are moved to `*.js`, so generally a good change overall. 3. The renderer binary needs a label reference to the JS entry point generated by the `ts_project()`, so this required some refactorings.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. `ts_project()` will use the given `tsconfig.json` file to determine the `module`. Since the base `tsconfig.json` in the repository is still using CommonJS, that means that *all* TypeScript will use CommonJS. This confuses Rollup when bundling client side scripts, so we need to tell client-side TypeScript compilations to use EcmaScript Modules instead of CommonJS modules. This new `tsconfig.client.json` serves that purpose. Any `ts_project()` for client-side scripts just needs to set `tsconfig = "//:tsconfig_client"`.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
…nt()` targets, because this would do nothing. Refs #50.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. This uses the standard `ts_project()`, **not** the workspace `ts_project()` macro since it is directly exposed to users. It now requires that `tsconfig` and `source_map` be explicitly passed in and examples needed to be updated. `declaration` is assumed to be `True`, since you can't compose multiple `prerender_component()` targets without it. Trickiest issue here was the declarative shadow DOM component. It doesn't use a real NPM package with subpath exports, so getting the `rules_prerender/declarative_shadow_dom` import to work is quite difficult. It seems that `js_library(package_name = "...")` does work, but this is more nuanced because declarative shadow DOM is a `prerender_component()`, **not** a simple `ts_project()`. As a result, it _must_ go into a `prerender_component(deps = [...])` as opposed to `lib_deps` like `//packages/rules_prerender`, even though both should be available under the `rules_prerender` package. The way I got this to work was by making two `js_library(package_name = "rules_prerender")` targets. One for the typical `rules_prerender` exports, and another for declarative shadow DOM. `rules_nodejs` is able to merge the two packages together and make them work as long as they map the `package_name` to the same Bazel package. This means I had to move the declarative shadow DOM re-export to `//packages/rules_prerender:declarative_shadow_dom`. This is still compatible with external use cases because they use a separate `:declarative_shadow_dom` target in the published workspace root which also aligns NPM package name -> Bazel package semantics.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. Mostly just works, but two major issues to overcome: 1. `ts_library()` generated ES6 sources which used ESM modules, but `ts_project()` just does what the `tsconfig.json` says, which is CommonJS by default. Need to use a separate `tsconfig` attribute for client-side scripts so they generate ESM JS which can be properly bundled by Rollup. 2. The `ts_project()` macro uses `link_workspace_root = True` by default which maps `rules_prerender` to `""` (the root package path). This is incompatible with `//packages/rules_prerender` which maps `rules_prerender` to `"packages/rules_prerender"` and cannot be used together. Test code which depends on `:%{component}_prerender_for_test` encounters this issue for reasons I don't remember. The easiest solution for now is to set `link_workspace_root = False` for such `ts_project()` targets and use relative imports. Long term we should move to a fully relative / package name import model. Filed #49 to follow up on that work.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
Refs #50. This is no longer blocked on `ts_library()`, however it still cannot be enabled as `rules_postcss` is broken. I don't fully understand the error or why it is an issue, but moving off `rules_postcss` should address it.
dgp1130
added a commit
that referenced
this issue
Jul 4, 2022
|
Successfully migrated everything to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ts_library()is not recommended and going away inrules_tsAFAICT. It also blocks moving off--bazel_patch_module_resolver.See investigation in #8.
The text was updated successfully, but these errors were encountered: