Skip to content

Obsolete Microsoft.AspNetCore.SpaServices and Microsoft.AspNetCore.No… #12892

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

Merged
merged 5 commits into from
Aug 9, 2019

Conversation

ryanbrandenburg
Copy link
Contributor

@ryanbrandenburg ryanbrandenburg commented Aug 5, 2019

Fixes #11627.

Description
Pursuant to aspnet/Announcements#379 we are marking the following packages as obsolete:

  • Microsoft.AspNetCore.SpaServices
  • Microsoft.AspNetCore.NodeServices

Customer impact
Customers using those packages will have to switch to Microsoft.AspNetCore.SpaServices.Extensions.

Regression?
No.

Risk
Low. While it's not likely that Obsoleting a class could cause a "bug" in the traditional sense, the migration path to Microsoft.AspNetCore.SpaServices.Extensions is not so direct as "If you're using X method switch to Y". Instead developers will have to interact with the various CLIs directly which could lead to some confusion on their part.

@mkArtakMSFT mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. labels Aug 5, 2019
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Some of the [Obsolete] attributes might not be needed, but overall great!

@mkArtakMSFT
Copy link
Contributor

Approved by tactics

@ryanbrandenburg ryanbrandenburg force-pushed the rybrande/SPAServicesCleaning branch from 4592075 to 718904f Compare August 6, 2019 22:11
@mkArtakMSFT mkArtakMSFT added the breaking-change This issue / pr will introduce a breaking change, when resolved / merged. label Aug 7, 2019
@pranavkm
Copy link
Contributor

pranavkm commented Aug 8, 2019

@ryanbrandenburg could you try rebasing on 3.0? We skipped a couple of the flaky tests you're seeing here.

@ElanHasson
Copy link
Contributor

Is there an example on how to replace calls to UseSpaPrerendering?

@@ -9,6 +12,7 @@ namespace Microsoft.AspNetCore.NodeServices
/// might change over time (e.g., the process might be restarted), the <see cref="INodeServices"/> instance
/// will remain constant.
/// </summary>
[Obsolete("Use Microsoft.AspNetCore.SpaServices.Extensions")]
Copy link

@eouw0o83hf eouw0o83hf Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no INodeServices in Microsoft.AspNetCore.SpaServices.Extensions - what are consumers supposed to be use instead?

Comment on lines +99 to 102
[System.ObsoleteAttribute("Use Microsoft.AspNetCore.SpaServices.Extensions")]
public static void AddNodeServices(this Microsoft.Extensions.DependencyInjection.IServiceCollection serviceCollection) { }
[System.ObsoleteAttribute("Use Microsoft.AspNetCore.SpaServices.Extensions")]
public static void AddNodeServices(this Microsoft.Extensions.DependencyInjection.IServiceCollection serviceCollection, System.Action<Microsoft.AspNetCore.NodeServices.NodeServicesOptions> setupAction) { }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above - there is no analog in Microsoft.AspNetCore.SpaServices.Extensions. What should be used instead?

@dougbu dougbu deleted the rybrande/SPAServicesCleaning branch May 18, 2020 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. breaking-change This issue / pr will introduce a breaking change, when resolved / merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants