Use minified .js files from submodule in source-build

[wtgodbe]

Fixes #56472. CC @dotnet/distro-maintainers

The build now uploads the minified .js files as a build artifact to make updating the submodule easier

[MichaelSimons]

How is this going to work in the product source build. I was naively expecting BuildNodeJS to be defaulted to false when DotNetBuildSourceOnly is true. Also I see there are a few places that specify the following condition which I wasn't expecting would be true anymore:

Condition="'$(BuildNodeJS)' == 'true' and '$(DotNetBuildSourceOnly)' == 'true'"

[wtgodbe]

How is this going to work in the product source build. I was naively expecting BuildNodeJS to be defaulted to false when DotNetBuildSourceOnly is true. Also I see there are a few places that specify the following condition which I wasn't expecting would be true anymore:

I was assuming we'd modify the product source build script to pass --no-build-nodejs like we used to. I prefer the broader/more generic condition of "Use the submodule if we're not building nodejs" to "use the submodule in source build"

[wtgodbe]

Also I see there are a few places that specify the following condition which I wasn't expecting would be true anymore:

Condition="'$(BuildNodeJS)' == 'true' and '$(DotNetBuildSourceOnly)' == 'true'"

As things currently stand we could remove that the BuildNodeJs part of that, but keeping it is more future-proof for when we eventually turn node builds back on in source build

[wtgodbe]

Although I can see the argument for I was naively expecting BuildNodeJS to be defaulted to false when DotNetBuildSourceOnly is true, so that everything is controlled from within this repo. Do you prefer that?

[MichaelSimons]

Also I see there are a few places that specify the following condition which I wasn't expecting would be true anymore:

Condition="'$(BuildNodeJS)' == 'true' and '$(DotNetBuildSourceOnly)' == 'true'"

As things currently stand we could remove that the BuildNodeJs part of that, but keeping it is more future-proof for when we eventually turn node builds back on in source build

That would work. I generally prefer encapsulating this within the repo as then it just needs to be defined in one place as to what the source-build behavior is. Without this you are duplicating this in your source-build CI legs as well as the product source-build. Ideally build.sh -sb at the repo level would behave the same or as close to as possible to the product source-build.

[MichaelSimons]

Although I can see the argument for I was naively expecting BuildNodeJS to be defaulted to false when DotNetBuildSourceOnly is true, so that everything is controlled from within this repo. Do you prefer that?

Yes - I think our posts overlapped :)

[MichaelSimons]

Also I see there are a few places that specify the following condition which I wasn't expecting would be true anymore:

Condition="'$(BuildNodeJS)' == 'true' and '$(DotNetBuildSourceOnly)' == 'true'"

As things currently stand we could remove that the BuildNodeJs part of that, but keeping it is more future-proof for when we eventually turn node builds back on in source build

I get your viewpoint. IMO it makes the current source confusing though as this condition would/should never happen.

[wtgodbe]

Updated to set BuildNodeJs based off of DotNetBuildSourceOnly. I prefer keeping the redundant condition to make it less likely we'll forget/mess something up when we come back to this in 10

[omajid]

Do you plan to drop the Node-Externals submodule from aspnetcore once this is merged in? Or is that useful for non-source-build development/releases?

[wtgodbe]

Do you plan to drop the Node-Externals submodule from aspnetcore once this is merged in? Or is that useful for non-source-build development/releases?

I'll defer to @javiercn on that one - I'd be fine going back to an online restore for the non-source build case, if there isn't a strong reason to keep Node-Externals

[javiercn]

Do you plan to drop the Node-Externals submodule from aspnetcore once this is merged in? Or is that useful for non-source-build development/releases?

I'll defer to @javiercn on that one - I'd be fine going back to an online restore for the non-source build case, if there isn't a strong reason to keep Node-Externals

We can remove the cache submodule; it'll simplify things if we don't need it.