rules_js icon indicating copy to clipboard operation
rules_js copied to clipboard

2.0 RELEASE

Open gregmagolan opened this issue 1 year ago • 6 comments

rules_js 2.x branch: https://github.com/aspect-build/rules_js/tree/2.x

FEATURES

  • perf: reduce size of npm_package_store transitive_files depset (#1594)
  • feat: add support for linking js_library as 1p npm deps (#1646)
  • feat: split js_image_layer into fine grained layers (https://github.com/aspect-build/rules_js/pull/1712)
  • feat: support pnpm v9 (https://github.com/aspect-build/rules_js/pull/1679)
  • feat: support pnpm.onlyBuiltDependencies (https://github.com/aspect-build/rules_js/pull/1723)

RCs ARE OUT

These are ready to try,

https://github.com/aspect-build/rules_js/releases/tag/v2.0.0-rc2

https://github.com/aspect-build/rules_cypress/releases/tag/v0.6.0-rc0 https://github.com/aspect-build/rules_esbuild/releases/tag/v0.21.0-rc1 https://github.com/aspect-build/rules_jasmine/releases/tag/v2.0.0-rc0 https://github.com/aspect-build/rules_jest/releases/tag/v0.22.0-rc0 https://github.com/aspect-build/rules_rollup/releases/tag/v2.0.0-rc0 https://github.com/aspect-build/rules_swc/releases/tag/v2.0.0-rc0 https://github.com/aspect-build/rules_ts/releases/tag/v3.0.0-rc0 https://github.com/aspect-build/rules_webpack/releases/tag/v0.16.0-rc0

BLOCKING TODOS (before 2.0 final)

  • [x] https://github.com/aspect-build/rules_js/issues/1445 / https://github.com/aspect-build/rules_js/pull/1701
  • [x] https://github.com/aspect-build/rules_js/issues/1641
  • [x] https://github.com/aspect-build/rules_js/pull/1694
  • [x] https://github.com/aspect-build/rules_js/pull/1690
  • [x] https://github.com/aspect-build/rules_js/pull/1701
  • [x] https://github.com/aspect-build/rules_js/issues/1652
  • [x] https://github.com/aspect-build/rules_js/issues/1689
  • [x] https://github.com/aspect-build/rules_js/pull/1731
  • [x] https://github.com/aspect-build/rules_js/pull/1734
  • [x] https://github.com/aspect-build/rules_js/pull/1730
  • [x] https://github.com/aspect-build/rules_js/issues/1753.
  • [x] https://github.com/aspect-build/rules_js/issues/1530
  • [x] https://github.com/bazelbuild/rules_nodejs/issues/3722
  • [x] https://github.com/aspect-build/rules_js/issues/1637

PUNTED TO POST 2.0 RELEASE (non-breaking follow-ups)

  • [ ] https://github.com/aspect-build/rules_ts/pull/569
  • [ ] https://github.com/aspect-build/rules_js/issues/1562
  • [ ] https://github.com/aspect-build/rules_js/issues/1563
  • [ ] add "strict" mode where npm_import fails if there are lifecycle scripts in the package.json and the user lifecycle hooks are not enabled or explicitly disabled on that package
  • [ ] add checks that the npm_translate_lock lifecycle_hooks and patches-like attributes do not contain entries that do not map to any npm packages

DOWNSTREAM RULE SET CHANGES

  • 🟢 rules_cypress : https://github.com/aspect-build/rules_cypress/pull/68
  • 🟢 rules_esbuild : https://github.com/aspect-build/rules_esbuild/pull/199
  • 🟢 rules_jasmine : https://github.com/aspect-build/rules_jasmine/pull/144
  • 🟢 rules_jest : https://github.com/aspect-build/rules_jest/pull/269
  • 🟢 rules_rollup : https://github.com/aspect-build/rules_rollup/pull/95
  • 🟢 rules_swc : https://github.com/aspect-build/rules_swc/pull/253
  • 🟢 rules_terser : https://github.com/aspect-build/rules_terser/pull/86
  • 🟢 rules_ts : https://github.com/aspect-build/rules_ts/pull/593
  • 🟢 rules_webpack : https://github.com/aspect-build/rules_webpack/pull/146

BREAKING CHANGES

User facing changes

npm_package

  • refactor: default include_runfiles to False in npm_package (#1567)

npm_translate_lock

  • refactor: change default linked target name to pkg (#1684)
  • refactor: remove unused warn_on_unqualified_tarball_url from npm_translate_lock (#1568)
  • refactor: remote deprecated package_json attribute from npm_translate_lock (#1569)
  • refactor: remove deprecated update_pnpm_lock_node_toolchain_prefix from npm_translate_lock (#1574)
  • refactor: remove pnpm_version from npm_translate_lock bzlmod extension (#1576)
  • refactor: remove npm_translate_lock(use_starlark_yaml_parser) (#1658)

npm_import

  • refactor: remove deprecated run_lifecycle_hooks from npm_import (#1572)
  • refactor: remove deprecated lifecycle_hooks_no_sandbox from npm_import (#1573)

js_filegroup => js_info_files

  • refactor: add include_sources and include_transitive_declarations to js_filegroup rule and gather_files_from_js_providers + gather_runfiles helpers (#1585)
  • refactor: rename js_filegroup to js_info_files (#1615)

declarations => types

  • refactor: rename declarations to types in JsInfo & throughout rules_js (#1619)

WORKSPACE

  • refactor: move toolchain registration call to new rules_js_register_toolchains WORKSPACE function (#1593)
  • refactor: removed deprecated //npm:npm_import.bzl (#1570)

Dependencies

  • refactor: set minimum rules_nodejs version to 6.1.0 (#1579)
  • refactor: set minimum aspect_bazel_lib to 2.7.1 (#1580)

Drop Bazel 5 support

  • refactor: drop support for Bazel 5 (#1589)
  • refactor: remove js_image_layer bazel 5 legacy symlink detection (#1610)

Other

  • refactor: remove expand_template rule since we now have a better alternative in bazel-lib (#1587)
  • refactor: remove legacy update_pnpm_lock default value (#1624)
  • refactor: don't gather files from NpmPackageStoreInfo providers in gather_files_from_js_info (#1663)
  • refactor: add include_sources and include_transitive_types attributes to js_binary, js_test and js_run_binary

Internal changes that are BREAKING for downstream rule sets but not for end users

js_binary

  • refactor: make internal unresolved_symlinks_enabled attribute of js_binary mandatory (#1571)

NpmPackageInfo

  • refactor: rename directory attribute of NpmPackageInfo to src (#1575)

NpmPackageStoreInfo

  • refactor: remove unused src_directory from NpmPackageStoreInfo (#1566)

JsInfo

  • refactor: rename JsInfo npm_package_store_deps to npm_package_store_infos (#1620)
  • refactor: rename include_npm_linked_packages to include_npm_sources && JsInfo npm_linked_packages to npm_sources (#1623)
  • refactor: rename declarations to types in JsInfo & throughout rules_js (#1619)
  • refactor: re-order fields in JsInfo for readability (#1648)

Other

  • refactor: rename gather_files_from_js_providers to gather_files_from_js_info (#1617)
  • refactor: rename gather_files_from_js_info to gather_files_from_js_infos (#1665)
  • refactor: remove utils.home_directory and use get_home_directory from Aspect bazel-lib utils instead (#1606)
  • refactor: remove //js:enable_runfiles and use @aspect_bazel_lib//lib:enable_runfiles instead (#1605)
  • fix: drop .sh extension from js_binary, merger and js_image_layer launchers (#1586)
  • refactor: remove unused NpmLinkedPackageInfo provider and corresponding unused npm_linked_packages from JsInfo; rename load bearing npm_linked_package_files to npm_linked_packages (#1588)
  • refactor: remove deprecated //js/private:enable_runfiles and //js/private:experimental_allow_unresolved_symlinks (#1577)
  • refactor: remove JS_LIBRARY_DATA_ATTR and DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING from js_helpers

Places to update when release is out

  • [ ] https://aspect-build.slack.com/archives/C03LLCZPA3D/p1713803872978319?thread_ts=1708649824.918659&cid=C03LLCZPA3D (internal slack thread)
  • [ ] https://github.com/fremtind/rules_vitest/issues/14

gregmagolan avatar Apr 20 '24 21:04 gregmagolan

cc @jbedard

gregmagolan avatar Apr 20 '24 21:04 gregmagolan

I know the 2.0 release is still in alpha, but 1st party npm dependencies are failing to me on RBE. The following target builds sucessfully on RBE on rules_js 1.x:

npm_link_all_packages(name = "node_modules")

ts_project(
    name = "compile_ts",
    srcs = glob(
        [
            "src/**/*.ts",
        ],
        exclude = [
            "src/**/*.spec.ts",
        ],
    ),
    declaration = True,
    root_dir = "src",
    source_map = True,
    transpiler = partial.make(
        swc,
        root_dir = "src",
        source_maps = "true",
        swcrc = "//backend:.swcrc",
    ),
    deps = [":node_modules"],
)

npm_package(
    name = "pkg",
    srcs = [":compile_ts"],
    include_runfiles = False,
    visibility = ["//visibility:public"],
)

The change of the default value for include_runfiles didn't affect me since I was already providing False before. The only change I made was in the target name (to pkg).

The target builds sucessfully without RBE. If I set include_runfiles = True it works with RBE too. But I suspect this is wrong, since I've been using False since rules_js 1.x where it works.

The error I get is:

Copying files to directory backend/packages/logging/pkg failed: (Exit 1): copy_to_directory failed: error executing CopyToDirectory command (from target //backend/packages/logging:pkg) 
  (cd /home/andre/.cache/bazel/_bazel_andre/f5cfe5f2674b3facfa2b14436fac5b0a/execroot/_main && \
  exec env - \
  external/aspect_bazel_lib~~toolchains~copy_to_directory_linux_arm64/copy_to_directory bazel-out/aarch64-fastbuild/bin/backend/packages/logging/pkg_config.json '')
# Configuration: a1829c87b28b44c4110093c03e1e47fc8ba5855812d3fc30aa05e0cb682bc1bf
# Execution platform: @@local_config_platform//:host
2024/05/15 00:24:05 lstat bazel-out/aarch64-fastbuild/bin/backend/packages/logging/node_modules/@opentelemetry/api failed: lstat bazel-out/aarch64-fastbuild/bin/backend/node_modules/.aspect_rules_js/@[email protected]/node_modules/@opentelemetry/api: no such file or directory

The @opentelemetry/api is a dependency declared on the package.json.

bazaglia avatar May 15 '24 00:05 bazaglia

The change of the default value for include_runfiles didn't affect me since I was already providing False before. The only change I made was in the target name (to pkg).

The target builds sucessfully without RBE. If I set include_runfiles = True it works with RBE too. But I suspect this is wrong, since I've been using False since rules_js 1.x where it works.

Interesting. Thanks for the report. I'm not sure off hand what could be different tho its may be related to npm package sources no longer being in the JsInfo types where previously they were. The change was actually on the 1.x branch: https://github.com/aspect-build/rules_js/pull/1554. Can you check if you also see the same issue on the latest 1.x release?

It might be that that change combined with some other change on the 2.x branch leads to the issue you're seeing. Either way, the fact that it is trying to copy a file from an npm package that is not there is probably a bug in the ruleset. The lstat is coming from the copy_to_directory golang tool. Looks like from the Realpath function in there: https://github.com/aspect-build/bazel-lib/blob/474e680caddadb8f95074406a42526b46c3e2cf1/tools/common/file.go#L35.

gregmagolan avatar May 15 '24 01:05 gregmagolan

@bazaglia Can you compare the generated {name}_config.json file from the 1.x working case and the 2.x broken case? The copy_to_directory/npm_package rule generates that file as a manifest of what to copy.

https://github.com/aspect-build/bazel-lib/blob/474e680caddadb8f95074406a42526b46c3e2cf1/lib/private/copy_to_directory.bzl#L481

I'd be interested to see the difference if you're able to send me those files over Bazel Slack.

gregmagolan avatar May 15 '24 01:05 gregmagolan

@gregmagolan You are right, there are some differences in the generated pkg_config.json file. I compared with rules_js 1.41.1 so #1554 is already included in there. It didn't seem to be the cause of the issue. I sent you the files on Slack.

bazaglia avatar May 15 '24 09:05 bazaglia

@gregmagolan You are right, there are some differences in the generated pkg_config.json file. I compared with rules_js 1.41.1 so #1554 is already included in there. It didn't seem to be the cause of the issue. I sent you the files on Slack.

I was able to repro and I have a fix for a case which looks like what you're hitting: https://github.com/aspect-build/rules_js/pull/1762

gregmagolan avatar May 28 '24 06:05 gregmagolan

Marking this closed as the 2.0.0 release is now published

alexeagle avatar Aug 15 '24 02:08 alexeagle