Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future plans of the concatjs devserver #2904

Open
devversion opened this issue Aug 31, 2021 · 3 comments
Open

Future plans of the concatjs devserver #2904

devversion opened this issue Aug 31, 2021 · 3 comments

Comments

@devversion
Copy link
Contributor

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.

filegroup(
name = "devserver_linux_s390x",
srcs = ["devserver-linux_s390x"],
# Don't build on CI
tags = ["manual"],
visibility = ["//visibility:public"],
)
filegroup(
name = "devserver_windows_amd64",
srcs = ["devserver-windows_x64.exe"],
# Don't build on CI
tags = ["manual"],
visibility = ["//visibility:public"],
)
filegroup(
name = "devserver_linux_ppc64le",
srcs = ["devserver-linux_ppc64le"],
# Don't build on CI
tags = ["manual"],

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.

@alexeagle
Copy link
Collaborator

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
Copy link
Collaborator

alexeagle commented Sep 4, 2021

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.

#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. #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.

@devversion
Copy link
Contributor Author

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.

@alexeagle alexeagle added this to the 5.0 milestone Oct 14, 2021
@alexeagle alexeagle assigned mattem and mrmeku and unassigned mattem Dec 7, 2021
@mattem mattem removed this from the 5.0 milestone Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants