rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

Future plans of the concatjs devserver

Open devversion opened this issue 2 years ago • 3 comments

Hey team,

this is more of a question than an issue, but I wanted to check on the maintenance of the concatjs devserver that is shipped as part of @bazel/concatjs.

Previously, when the devserver still was part of rules_typescript, it was extremely difficult to land changes (like the windows support in the past) because the devserver implementation was synced into g3 and the team was hesitant to make any changes.

Given that the TypeScript rules are now vendored, I wonder how/if the devserver is considered being maintained by rules_nodejs, especially since support for new platforms has been added in the past to rules_nodejs, but corresponding devserver binaries for these platforms have not been shipped. i.e. darwin arm64 for M1, linux_s390x, powerpc etc.

https://github.com/bazelbuild/rules_nodejs/blob/9b454e38f7e2bbc64f75ee9a7dcb6ff45f1c7a12/packages/concatjs/devserver/BUILD.bazel#L58-L78

The filegroups have been defined for these platforms supported in rules_nodejs, but the actual binaries for example are not built in the vendored rules_typescript repository.. so there is actually no support for the concatjs devserver.

It's worth asking the question: is it worth continuing to maintain the Go-based concatjs devserver, or would a more JS-based devserver be suitable (like we have built in the angular/components repo)? Such devservers could use e.g. browser-sync under the hood and allow for a lot of additional benefits.

devversion avatar Aug 31 '21 17:08 devversion

answers at a couple different levels:

  1. I just found that not only are we limited to the three platforms, but we're also publishing the devserver binaries in both @bazel/typescript and @bazel/concatjs so that's wasting a lot of bytes. The correct fix, which I'll work on now, is to publish the concatjs go binaries on our github releases page and not in the npm packages. Then add a toolchain definition to fetch them like we have for esbuild and cypress. Adding the missing platform binaries is then trivial. This should be in 4.1
  2. Long-term we would love to drop concatjs support from rules_nodejs to reduce our scope and OSS maintenance burden. I'm just not sure where that would go, and who would support it. I don't want to leave users stranded. rules_closure is the place where Google-style JS lives but I imagine it's unlikely they would take it. Some answer for this on the 5.0 release would be nice
  3. Using some JS-based devserver sounds great, but that certainly wouldn't go in rules_nodejs to begin with, so I don't have much to say :)

alexeagle avatar Aug 31 '21 21:08 alexeagle

I'm going to need some help to get this over the finish line. Either someone with time to finish it or a corporate user to fund the work.

https://github.com/bazelbuild/rules_nodejs/pull/2914 is WIP that would publish concatjs binaries on our releases and consume them as a toolchain

then I think we need to upgrade rules_go, as the darwin_arm64 binary is very small so I suspect non-functional. https://github.com/bazelbuild/rules_nodejs/pull/2905 is a start on that, however there was a breaking change to the Runfiles library a long time ago which we never accounted for, and is patched out. We basically got stuck on a fork. So I think we'll need to vendor in some of the code or refactor the devserver.

alexeagle avatar Sep 04 '21 18:09 alexeagle

Agreed 👍 I've replied on the toolchain start PR you created.

Using some JS-based devserver sounds great, but that certainly wouldn't go in rules_nodejs to begin with, so I don't have much to say :)

I'd argue that a devserver provided here would make sense because setting up devservers for Bazel w/ NodeJS requires logic for runfile resolution, so having one here seems useful. Surely this could be maintained by a third party but worth considering a simple JS based one. I don't feel strongly though.

devversion avatar Sep 04 '21 18:09 devversion