Skip to content
This repository was archived by the owner on Feb 25, 2021. It is now read-only.

[WIP] local and session storage support as nupkg #205

Closed
wants to merge 2 commits into from

Conversation

LunicLynx
Copy link
Contributor

@LunicLynx LunicLynx commented Mar 3, 2018

Move storage implementation into its own project to create a nupkg as suggested here #201 by #201 (comment)

Explanation:

The BasicTestApp now has a reference to Microsoft.AspNetCore.Blazor.Browser.Storage.

Build Phase

When building the ReferencedAssemblyFileProvider discovers JavaScript files that are embedded as resources.
After that the IndexHtmlFileProvider writes script tags for every javascript file found.
Which concludes the modifications during the build phase.

Execution Phase

The JavaScript parts will be loaded as soon as the script tags are discovered by the browser. To use the extension from c# it is only required to add a reference and then add the services to DI Like so.

var serviceProvider = new BrowserServiceProvider(configure =>
            {
                configure.AddStorage();
            });

Missing

Actual nupkg creation. How does one actually do this? 😄

@SteveSandersonMS is this what you had in mind?

@LunicLynx LunicLynx force-pushed the storage-nupkg branch 2 times, most recently from 0de68ff to 66fe67b Compare March 5, 2018 23:06
@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Mar 6, 2018

@LunicLynx This is superb - thanks so much for this!

There are a few tweaks I'd recommend, which I'll get back to you about in a few days (got a lot of urgent stuff to focus on right now), but overall this looks extremely useful. I just tried it out and it worked great. Also I'll answer your question about how to make a NuGet package from this.

Thanks again!

@LunicLynx
Copy link
Contributor Author

LunicLynx commented Mar 6, 2018

Update:
Removed Commented out import
Removed duplicate storage.ts
Added EOF line breaks
Rebase on dev branch

@SteveSandersonMS
Copy link
Member

Great stuff @LunicLynx. I'll be able to focus more on this tomorrow/Friday and we can hopefully figure out the final details.

@SteveSandersonMS
Copy link
Member

Update: In #247, I'm adding support for importing JS/CSS and other static content files from NuGet packages. This should hopefully cover the requirements for this package plus others.

Instead of doing it as embedded resources as in this PR, I went for regular content files on disk. The main benefit is that it means we're not shipping the static content to the client twice (once in the .dll, and a second time via a <script> tag etc.). I know that IL stripping would eliminate the embedded resources anyway, but in case there are packages with huge static resources it's still preferable not to include them in the .dll in development mode.

@LunicLynx Once #247 is merged I'd love to work with you to finalise what would be in your localStorage package.

@LunicLynx
Copy link
Contributor Author

@SteveSandersonMS so how are we going to do this? Just ping me on gitter?

@LunicLynx
Copy link
Contributor Author

LunicLynx commented Mar 14, 2018

This is how far i got. Not quite sure why it isn't working. But the TestContentPkg doesn't work for me either. 🤔

Not Working, as in, not working in the BasicTestApp.

@SteveSandersonMS
Copy link
Member

@LunicLynx Sorry I'm being a bit delayed on this. We're at full throttle trying to get the 0.1.0 public release finalised this week, so it will probably be next week when I can address this. Hope that's OK. The new built-in mechanism for including content from NuGet packages will work well for your localStorage package, though :)

@LunicLynx LunicLynx force-pushed the storage-nupkg branch 3 times, most recently from 1385cbb to a6adbff Compare March 17, 2018 12:11
@LunicLynx
Copy link
Contributor Author

LunicLynx commented Mar 20, 2018

On hold until #247 (comment) gets addressed.

@LunicLynx
Copy link
Contributor Author

LunicLynx commented Apr 5, 2018

Rebased onto dev
@SteveSandersonMS Will this move forward?

@SteveSandersonMS
Copy link
Member

Yes, thanks for your patience! We've gone slowly with this because it's super important and we really want to get the mechanism right. As it happens it's what I've been working on today: #492

In the end we decided to go with your original design for embedding the content using <EmbeddedResource> in the assemblies, rather than my design where it used NuGet contentFiles. The reason is that:

  • The <EmbeddedResource> design means it works exactly the same whether your content package is a NuGet package referenced via <PackageReference> or a local source project on disk referenced via <ProjectReference>. You don't have to pack the project before you can consume it, or do any other weird MSBuild hacks.
  • My original concern about it bloating the assemblies no longer applies. Since that discussion, we'd enabled the IL linker on every build by default, which will automatically remove the embedded resources before the .dlls are served to the browser. So there's no longer any drawback.

So today or tomorrow we should merge #492 and then we'll have the final mechanism for content packages, which will be available in Blazor 0.2.0 which is planned to ship early next week. This should simplify your package too.

Does this sound OK?

@LunicLynx
Copy link
Contributor Author

Ok. Should I revert it back to the earlier version then?

Also, i was thinking that it could be useful to have almost all browser api’s in one package. What do you think?

@SteveSandersonMS
Copy link
Member

@LunicLynx Now we've released the concept of a Blazor library package (in a large part inspired by your design), would you be interested in making and publishing a "local storage" package based on that approach? This could ideally be done in a separate repo.

@SteveSandersonMS
Copy link
Member

Also, i was thinking that it could be useful to have almost all browser api’s in one package

It's possible that the Mono team is working on automating the creation of such a package, so it's probably not something you need to build by hand.

However there will certainly still be a good use case for standalone packages that expose specific APIs for people who don't want everything in one huge bundle. Hand-crafted APIs are always going to better match what C# devs are looking for rather than an automated conversion from JavaScript. So if you're interested in building a localstorage-specific package, there would be a good need for that.

@LunicLynx
Copy link
Contributor Author

@SteveSandersonMS I can absolutely do this. Can you give me a few pointers on how to create such a library project?

@Suchiman
Copy link
Contributor

Probably worth pointing out that there's https://github.com/chrissainty/BlazoredLocalStorage

@danroth27
Copy link
Member

@LunicLynx We actually now have a Blazor library project template that you can start with that handles dealing with the static assets. It includes some sample code for shipping a library that does JavaScript interop. See https://blogs.msdn.microsoft.com/webdev/2018/04/17/blazor-0-2-0-release-now-available/ for some details.

@LunicLynx
Copy link
Contributor Author

@SteveSandersonMS @danroth27 Ok, i think i got this right. And did put it here:
https://github.com/cloudcrate/BlazorStorage
Only question that remains where should i publish this package? Directly to nuget?

@danroth27
Copy link
Member

@LunicLynx Yes, publishing to nuget.org should be fine so that folks can try it out easily. It's not unexpected that there will be multiple implementations from the community published for the same browser functionality.

@LunicLynx
Copy link
Contributor Author

@LunicLynx LunicLynx closed this May 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants