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

Don't pull in entire ASP.NET framework #59

Closed
wants to merge 1 commit into from

Conversation

Farthom
Copy link
Contributor

@Farthom Farthom commented Apr 5, 2023

I've attempted to use this library in a background application responsible for processing GPS coordinates. Our application being a backend processor is NOT an AspNet application and that framework is not available in the production environment on which our application runs. The problem is, Geo.NET contains a hard reference to the AspNet framework. This means, simply adding a reference to Geo.NET causes the production services to fail to start. (Errors about missing AspNet framework)

I've poked through a bit, and it seems the only usage is "QueryString" class for building request Uris. This class exists in the Http.Abstractions package which is perfectly fine to reference even in non aspnet applications. I've not dug too deeply but I can't seem to see any good reason to be pulling in this entire framework. I've made a temporary one off build of this library encompassing the below changes and it is working great for us. Hopefully they can be merged into the official package.

@JustinCanton
Copy link
Owner

@Farthom, the usage of the Microsoft.AspNetCore.Http.Abstractions nuget has been abandon by Microsoft. It is an old netcoreapp2.2 nuget (as evidenced by the versioning of it).

The proper upgrade path for that nuget it to use a framework reference in newer versions of .net:
dotnet/aspnetcore#18408

Now, I can dig into other solutions for this problem, to try and see if there are any workarounds. But I would prefer not to maintain the usage of the Microsoft.AspNetCore.Http.Abstractions nuget in newer .net versions.

@JustinCanton
Copy link
Owner

Unfortunately even this ticket talks about how the code has been moved into .NET Core:
dotnet/aspnetcore#20946

They do mention that it is possible to use the older package, as long as it doesn't cause conflict. I am worried that it may cause conflicts for people consuming Geo.NET though.

I will need to think it over for a bit more. Another option is to just take the source code for the QueryString class and add the class locally.

@JustinCanton
Copy link
Owner

I have created #60 for this issue. At the moment, I am leaning towards pulling QueryString into the Geo.Core project and building from there.

@JustinCanton
Copy link
Owner

I have thought it over for a while, and I think the best thing to do here is to create a local copy of the QueryString class. I will be closing this PR and making a new PR with the changes.

@Farthom
Copy link
Contributor Author

Farthom commented Apr 6, 2023

Fantastic! I look forward to the next version I can use.

Thanks so much!

@JustinCanton
Copy link
Owner

No problem! I will merge the code tonight, and you should have an alpha build you can play with, if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants