-
Notifications
You must be signed in to change notification settings - Fork 230
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
Blazor CRUD scaffolding updates #2743
Comments
IGNORE (probably)VS is suggesting a primary ctor, so this code will go away. See my comment at ...
Scaffolding/src/Scaffolding/VS.Web.CG.EFCore/Templates/DbContext/NewLocalDbContext.cshtml Line 43 in d30f294
public @Model.DbContextTypeName (DbContextOptions<@Model.DbContextTypeName> options)
public @(Model.DbContextTypeName)(DbContextOptions<@Model.DbContextTypeName> options)
public BlazorWebAppMoviesContext (DbContextOptions<BlazorWebAppMoviesContext> options) |
More NITs 😈 ...
|
In the file ... ... the model isn't lowercased to match what's done for the other components of the set ... - @page "/<#= pluralModel #>/edit"
+ @page "/<#= pluralModelLowerInv #>/edit" Because of it, the path comes out (for example using a movies database example) ... @page "/Movies/edit" ... instead of the expected ... @page "/movies/edit" |
The following cross-links will need to be removed because
WRT documentation coverage: The Blazor Web App tutorial (in progress) will touch on overposting with a short section, and I think we're going to have a special Blazor static SSR form overposting section in our Blazor forms article coverage. If so, I'll open a follow-up issue on this repo to get a new link placed that points to the new coverage. UPDATEOpened ... Add Blazor forms overposting coverage ... to place Blazor-specific overposting coverage. After coverage goes live, I'll open a new scaffolder repo issue to get the Blazor cross-links into the Blazor bits here and remove the main doc set cross-link. The main doc set coverage will be linked via the Blazor coverage, so devs will still be able to reach it for more info. |
This comment was marked as resolved.
This comment was marked as resolved.
Unfortunately, I don't think we ever formalized our Blazor style guide, but in product code (templates, scaffolding, etc) I think we generally only prefix with |
There are also extra spaces in the Razor markup files of the CRUD components. |
Missing
- bool <#= modelName #>Exists(<#= primaryKeyShortTypeName #> <#= primaryKeyNameLowerInv #>)
+ private bool <#= modelName #>Exists(<#= primaryKeyShortTypeName #> <#= primaryKeyNameLowerInv #>) |
I'm not sure where this is located (probably over in the main repo actually), but there isn't a return on the last line of the I'll go look now and see if this is in the project template over there. BRB .................... The ... and this one ... Those are the only ones. Every other file is fine. I'm not sure where those are located, here or back on the main repo. I'll leave this comment here in case the PU engineer wants to hunt them down and put in a fix for them. |
"namespace" is misspelled in three spots ... https://github.com/search?q=repo%3Adotnet%2FScaffolding%20namesapce&type=code |
Not related to Blazor, but something that I'll pile on here WRT the The help list of options doesn't look right ...
... the last two ( |
Two of the components cross-link for more information on overposting ... // To protect from overposting attacks, see https://aka.ms/RazorPagesCRUD Cross-refs:
However, that's not a very helpful cross-link because it lands here ... ... with API and an approach that isn't available in a Razor component scenario. We now have a new overposting section in the Blazor Forms overview article. Update the https://learn.microsoft.com/aspnet/core/blazor/forms/#mitigate-overposting-attacks ... and I can't create an |
Hey @guardrex, thanks a lot for the incoming feedback, looking at it currently and fixing the templates accordingly. |
@deepchoudhery ... Nevermind on the comment that I left here. I forgot for a moment that we're changing our documentation descriptions and examples to avoid prefixing nonliterals with <QuickGrid Class="table" Items="DbFactory.CreateDbContext().{CLASS}"> ... is correct, no I'm still getting used to this because we've gone about five years the other way. Old habits are hard to break! 😄 |
All good, going to go through this issue again soon and catalog remaining changes into a comment for easier tracking. |
Found another one ... there's a blank line after the opening This extra line is in all of the component files: - @page "/movies"
-
- @using Microsoft.EntityFrameworkCore
- @using Microsoft.AspNetCore.Components.QuickGrid
- @using BlazorWebAppMovies.Models
- @inject IDbContextFactory<BlazorWebAppMovies.Data.BlazorWebAppMoviesContext> DbFactory
+ @page "/movies"
+ @using Microsoft.EntityFrameworkCore
+ @using Microsoft.AspNetCore.Components.QuickGrid
+ @using BlazorWebAppMovies.Models
+ @inject IDbContextFactory<BlazorWebAppMovies.Data.BlazorWebAppMoviesContext> DbFactory |
@danroth27 ... I'm not sure if we're proceeding with the tutorial updates because not all of the scaffolding updates are done yet. If they're worked piecemeal, I need to go back into the tutorial files repeatedly (or again after 9.0 GA) ... it's time consuming to repeatedly revisit it ... thus costly 💰. Nevermind on the next bit ... the packages updated right after I mentioned it ........
<QuickGrid Class="table" Items="DbFactory.CreateDbContext().Movie">
...
</QuickGrid>
|
One more ...... The docs favor naming the injected @inject NavigationManager Navigation
...
Navigation.XXXXX This would be a helpful change for consistency with the rest of the articles. |
I have another one ... In the @inject IDbContextFactory<BlazorWebAppMovies.Data.BlazorWebAppMoviesContext> DbFactory However, the component already uses @using BlazorWebAppMovies.Data ... so it would be best to 🔪 it from the context for ... @inject IDbContextFactory<BlazorWebAppMoviesContext> DbFactory This situation is only found in the |
sounds good, will get these 2 fixed in next servicing. Do they apply to both .NET 8 and 9 scenarios? |
Yes, both 8.0 and 9.0 for both of them. ... and I recall that you said the primary ctor change would take place later because it's a larger task that applies in more spots. |
ah yes sounds about right. i'll get all these in together tyty |
I'm going to place everything that I find in this one issue (several comments). If you want me 🦖 to PR the updates, I can do it after I'm done writing a large tutorial that makes use of this scaffolding in a few weeks.
Should the semicolons in theBlock
entries of the following file be there?Scaffolding/src/Scaffolding/VS.Web.CG.Mvc/Blazor/blazorWebCrudChanges.json
Line 10 in d30f294
Scaffolding/src/Scaffolding/VS.Web.CG.Mvc/Blazor/blazorWebCrudChanges.json
Line 17 in d30f294
I think I received a double semicolon (unless it was due to a stray keystroke on my part 😈) ...Nevermind! The double-semicolon seems to have been a stray keystroke on my part.
The text was updated successfully, but these errors were encountered: