Restore CI validation that blazor.server/webview.js are up-to-date

[SteveSandersonMS]

In the past we definitely used to have CI-time checking that blazor.server.js and blazor.webview.js are up-to-date in source control, i.e., that the committed versions of these files are byte-for-byte identical to what the CI gets if it builds src/Components/Web.JS. This checking is important because:

• We're still required to put these "binaries" in source control as a requirement of source build (it doesn't allow depending on the JS-based build tools)
• If we don't verify that the committed version is up-to-date, a developer could forget to update it and hence their changes wouldn't take effect, or a PR could even deliberately supply a JS "binary" that contains unexpected content that doesn't match the sources and we'd then ship it

I might be missing something, but it looks as if our up-to-dateness verification is no longer being enforced. A recent PR made a TypeScript change that definitely impacts the .js files, but didn't include updated .js files in the PR, and yet the build didn't fail with any error about that.

[BrennanConroy]

The CI builds the js binary and uses the one it built for shipping.

I think the main reason is that a build version is burnt into the file, which will change for every build so the CI will always have a different version than what is checked in. e.g. current file has 0.0.0-DEV_BUILD, but because the CI builds projects in parallel, it's possible for it to grab the SignalR files before they're modified, or after they've been modified with the version of 8.0.0-CI
#13106 (comment)

Other reasons: it's a pain when making changes for the SignalR TS client, and also it causes merge conflicts when forward porting or backporting.

[SteveSandersonMS]

@BrennanConroy Interesting. Could you clarify a bit further to make sure we're all in sync?

I think the main reason is that...

Main reason for what - that it should be checked for consistency, or that it should not be checked for consistency, or that this should or should not be regarded as a bug? Or main reason that "the CI builds the js binary and uses the one it built for shipping"?

Things I still don't understand:

• If the CI was able to use its own-built version in all cases, why would we still want to commit any version of the built file into the repo? Like you point out, it causes merge conflicts and all kinds of inconveniences. I thought that source build had no choice but to use the committed version, which would then be the wrong version if someone forgot to update it in the repo and the CI system doesn't check for this.
• In fix npm flake #13106 which you link to, I can see that CodeCheck.ps1 was updated in mid-2019 to exclude blazor.server.js from this check. But we didn't remember to update this to also exclude blazor.webview.js when that was added in for MAUI - I guess because nobody on the Blazor team remembered that this exclusion existed. So how come blazor.webview.js is not being checked for consistency?

[BrennanConroy]

Main reason for what

The main reason it's being skipped for consistency.

If the CI was able to use its own-built version in all cases, why would we still want to commit any version of the built file into the repo?
I thought that source build had no choice but to use the committed version

Right, that's why it's checked in. This whole thing is a hack to workaround source-build not having the ability to build the js file (at least not at the time, idk if things have changed).

which would then be the wrong version if someone forgot to update it in the repo and the CI system doesn't check for this

Yeah, that is a downside. I assume source-build runs tests? So it would find a problem if the js file wasn't updated?

So how come blazor.webview.js is not being checked for consistency?

No idea what that file is. If it isn't using the SignalR files then it probably doesn't have the same issue.

[dougbu]

I assume source-build runs tests?

No, source-build doesn't even build test projects.

---

A bit more is coming back to me: Source build placed restrictions on what / how we can build the repo and TS ➡️ JS was disallowed. @crummel @MichaelSimons is it possible these days to use the typescript compiler / yarn and so on in source-build❔

[SteveSandersonMS]

OK, thanks. It sounds like we just don't have a good solution here, in that:

1. We can't enforce that the file contents are consistent with sources, because the build inherently varies each time, because it embeds a build version in the SignalR .js output
2. But failing to enforce consistency puts us at risk of shipping an inconsistently-versioned file, or worse, shipping intentionally backdoored code that we couldn't notice in a PR review because it was embedded in a "binary"

It seems like a better solution is needed. This could be:

• Setting up a completely new CI job whose only role is to build the JS files in some version-independent way, for example by setting the version to 0.0.0-DEV_BUILD, and checking the repo binary matches what it gets from the sources.
• Or, making the consistency check smarter, and able to allow a specific token to vary (as long as it still matches a particular regex). Might be tricky to design this.
• Or, getting source build to be able to do TypeScript builds like @dougbu comments above.

[javiercn]

I think longer term sourcebuild needs to handle node/js. Right now, we are cheating by checking in the build outputs and that is precluding us from having consistent results.

Maybe we could divide and conquer here, although I believe it goes against the direction we plan to take build wise, it might be possible to have the regular build produce the artifacts on the flight, and when using source build, pick the artifacts from a separate repository that has the prebuilt artifacts checked-in.

That way, when a change happens, we do not have to check-in anything, and a CI process/bot can ensure that the assets get updated later on for source build.

[dougbu]

The source-build requirement is basically to avoid external "pre-builts". Checking the binary file in is cheating but, worse, it means the source-built files have the wrong version in them. Keeping the file updated somehow is fine but the current system already does that.

Would automating the process do more than address the merge conflicts❔ And, how would you handle embedded versions❔

In our own builds, we could update the JS files in one build job and use that artifact in the source-build job. But that wouldn't help an actual source build and therefore doesn't really solve the problem.

[crummel]

Specifically the source-build problem is that the way npm/yarn packaging works in the open-source world is that every single different npm/yarn dependency has to be packaged and shipped as a separate Fedora/RHEL/Ubuntu/etc package. The true-to-the-vision way of fixing this would be for each of these that we are dependent on to go through that process. If it would be interesting I can run the build with the available packages and see how many we're missing now - when we last checked it was on the order of ~30 missing dependencies including transitives.

cc @dotnet/distro-maintainers

[tmds]

Are we shipping this with source-built .NET? Or does it come with the wasm-tools workload that users acquire from nuget.org?

[javiercn]

@tmds this comes from source built .net. blazor.server.js is embedded in the Components.Server assembly.

@dougbu my understanding at the time was that we did not want to have to deal with JavaScript/NPM dependencies as part of source built. If that is no longer a constraint, then we should strive to make source build also build the JS dependencies and avoid having anything checked in.

@crummel does source build already handle JS/TS builds?

[tmds]

ok, then we're shipping it.

For source-built .NET, these libraries need to be built from source.

We're not building any JS/TS libraries so far.

[tmds]

@javiercn @dougbu what tooling is needed to build this from source?

cc @omajid

[omajid]

Like @crummel says, this was motivated by the fact that ASP.NET Core has a hard (and impossible to remove) dependency on typescript, and we wanted to ship .NET in offline environments as well as where node/npm/yarn and typescript were simply not available.

The options, as I saw them were:

1. Remove dependency on typescript.
2. Package all the needed npm/node/typscript packages (and their recursive dependencies) into each Linux that is interested in consuming .NET and building it from source
3. Pre-build this typescript code and bundle the resulting code into ASP.NET Core

Option 1 would be nicest from a distro-packager point of view. I understand it's a huge ask for the ASP.NET Core folks.

Option 2 is the "correct" approach but not quite doable in certain environments. For example, it was a no-go on RHEL 7 where we didn't have node, and packaging/maintaining it was simply too much overhead for adding .NET. The situation may be different now. Or, perhaps, we can enable building this from source on platforms where the necessary compilers and other npm packages might be available?

Option 3 seemed like the least of all evils, specially because we still have the sources (both typescript and the compiled javascript "source") and can patch them by hand if needed.

[tmds]

Another option could be to not have Blazor Server support on platforms that can't build this blob from source.
We dislike source build .NET not being able to deliver the same feature set as Microsoft .NET.
So maybe option 3 was indeed the least of all evils.

I'm not very familiar with the node ecosystem. If we do npm install won't it fetch all the java script source files we'd need to include? Or is it more complex than that?

[ayakael]

I'd like to explore option 2 on Alpine Linux. Is there a list of npm dependencies somewhere that I can crosscheck against already packaged libraries?

[omajid]

It seems like some Linux distributions are becoming more flexible with option 2 as well. I am re-reading https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/ which calls out that the current policy for Fedora is that "you should bundle all the nodejs libraries that are needed [into your package]".

[mkArtakMSFT]

@javiercn wouldn't this be covered as part of the work that you're doing already?

[javiercn]

This will be solved once #11592 is addressed.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[mkArtakMSFT]

Closing as a dupe of #11592