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

Try to make service worker publishing csproj nicer #19007

Closed
SteveSandersonMS opened this issue Feb 13, 2020 · 2 comments
Closed

Try to make service worker publishing csproj nicer #19007

SteveSandersonMS opened this issue Feb 13, 2020 · 2 comments
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly

Comments

@SteveSandersonMS
Copy link
Member

Within the PWA template, we have this:

  <ItemGroup>
    <!-- When publishing, swap service-worker.published.js in place of service-worker.js -->
    <Content Update="wwwroot\service-worker*.js" CopyToPublishDirectory="false" />
    <ContentWithTargetPath Include="wwwroot\service-worker.published.js">
      <CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory>
      <TargetPath>wwwroot\service-worker.js</TargetPath>
    </ContentWithTargetPath>
  </ItemGroup>

It's not absolutely outrageous, but it's not as elegant and approachable as we'd prefer things to be in newly-created projects. Are there any MSBuild/SDK experts who can suggest a cleaner way to achieve this? The requirements are:

  • When publishing, a certain <Content> item (for example wwwroot\service-worker.js) should not actually get included in the publish output
  • At the same time, a different <Content> item (for example, wwwroot\service-worker.published.js) should get published, but at the path originally occupied by the first item (e.g., wwwroot\service-worker.js)

Any suggestions gratefully received! cc @rainersigwald @nguerrera

Appendix: Things I've considered / tried

1. Same mechanism, different phrasing

I know we could "simplify" the above slightly by merging more things onto the same line (e.g., converting TargetPath to an attribute) but the I think that obscures things further given how long the lines would end up being.

2. Different mechanism with pure msbuild

<ItemGroup>
    <Content Update="wwwroot\service-worker.js" CopyToPublishDirectory="false" />
    <Content Update="wwwroot\service-worker.published.js" TargetPath="wwwroot\service-worker.js" />
</ItemGroup>

This looks nice and understandable, but doesn't work because the AssignTargetPath task doesn't retain existing TargetPath metadata. It just overwrites that metadata indiscriminately.

However AssignTargetPath does respect Link. We can't set that statically because it would mess up the layout in Solution Explorer, but I did try setting it inside a target like this:

<Target Name="PublishServiceWorker" BeforeTargets="CoreBuild">
    <ItemGroup>
        <Content Update="wwwroot\service-worker.js" CopyToPublishDirectory="false" />
        <Content Update="wwwroot\service-worker.published.js" Link="wwwroot\service-worker.js" />
    </ItemGroup>
</Target>

However that doesn't work because of this unexpected behavior. It assignes the Link to all Content items ignoring the Update condition.

3. Factoring it out into SDK

I'm aware we could define new primitives in the SDK. In the short term, that would mean "in the Blazor build targets" but in 5.0 that becomes part of the web SDK generally.

For example, we could have user syntax like this:

<ItemGroup>
    <ServiceWorker Include="wwwroot\service-worker.js" PublishContentFrom="wwwroot\service-worker.published.js" />
</ItemGroup>

We would then put some logic in Blazor's publish targets to find such items and modify the set of ContentWithTargetPath items accordingly. I'm not keen on this because:

  1. It's new concepts for us to document and for people to understand. They don't really know what magic behaviors are triggered by this new item type. People have no way to know, for example, if it's valid to publish a single file as a service worker without doing this "replace on publish" step.
  2. People will come up with reasons for wanting more control, for example swapping in content that was generated at build time via Webpack. Unless people actually control the underlying logic, they won't have a practical way to make things happen at different points in the build/publish process.
  3. It's claiming to be specific to service workers, but actually is a completely general "replace on publish" feature that people might end up using for unexpected different reasons. (Even worse would be calling it <ReplaceOnPublish ... /> because something that general has no business being in the Blazor or web SDKs.)
  4. The PublishContentFrom notion implies we can find the corresponding <Content> item and either update it or make it not get double-written. This is a risk because if people phrase the itemspec in a different way from its underlying representation (e.g., using an absolute path), it won't work without more subtle logic.

Another possible user syntax is:

<ItemGroup>
    <Content Update="wwwroot\service-worker.js" CopyToPublishDirectory="false" />
    <Content Update="wwwroot\service-worker.published.js" PublishAs="wwwroot\service-worker.js" />
</ItemGroup>

Again, this looks quite nice, but defining a new low-level primitive like this in the Blazor/web SDKs is hazardous. People will try to use it for totally different purposes, and be confused why it doesn't work the same in non-Blazor projects.

@SteveSandersonMS SteveSandersonMS added area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Feb 13, 2020
@rainersigwald
Copy link
Member

However that doesn't work because of this unexpected behavior. It assignes the Link to all Content items ignoring the Update condition.

Try this instead:

<Target Name="PublishServiceWorker" BeforeTargets="CoreBuild">
    <ItemGroup>
        <Content Condition=" '%(Identity)' == 'wwwroot\service-worker.js' " CopyToPublishDirectory="false" />
        <Content Condition=" '%(Identity)' == 'wwwroot\service-worker.published.js' " Link="wwwroot\service-worker.js" />
    </ItemGroup>
</Target>

@SteveSandersonMS
Copy link
Member Author

Thanks for the suggestion, @rainersigwald !

Since this was posted, @javiercn and I looked at it again a few days ago and came up with a pattern we preferred due to it not involving creating a new target:

  <ItemGroup Condition="'$(DesignTimeBuild)' != 'true'">
    <Content Remove="wwwroot\service-worker.js" />
    <Content Update="wwwroot\service-worker.published.js" Link="wwwroot\service-worker.js" />
  </ItemGroup>

We could change the use of $(DesignTimeBuild) to a different property if there's another one that's a more natural fit and has the same effect, i.e., doesn't apply the Link values statically since that would confuse people looking at Solution Explorer in VS.

I'll close this issue since I think we've got a satisfactory solution now. But suggestions for improvement are still welcome!

@SteveSandersonMS SteveSandersonMS added the Done This issue has been fixed label Feb 20, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly
Projects
None yet
Development

No branches or pull requests

3 participants