aspnetcore-tooling migration #21581
Merged
aspnetcore-tooling migration #21581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
\n\nCommit migrated from dotnet/razor@c5a7e9e
Fix invalid cast in non-generic parameterized ChildContent\n\nCommit migrated from dotnet/razor@e622d7c
\n\nCommit migrated from dotnet/razor@3fe99e9
\n\nCommit migrated from dotnet/razor@627710f
…ferences (dotnet/razor#560) \n\nCommit migrated from dotnet/razor@e5a23e0
* Fix invalid cast in non-generic parameterized ChildContent * Fix crash in functions block \n\nCommit migrated from dotnet/razor@0acc18b
# Conflicts: # src/Razor/src/Microsoft.AspNetCore.Razor.Language/Extensions/FunctionsDirectivePass.cs # src/Razor/test/RazorLanguage.Test/Extensions/FunctionsDirectivePassTest.cs \n\nCommit migrated from dotnet/razor@de4b0c7
- Diagnostics were not being raised to the `RazorSyntaxTree` and weren't being ordered on final output. This resulted in some of our tests missing the fact that certain cases were generating errors. - Made all three phases of Razor parsing order their diagnostics. - Added `GetAllDiagnostics` methods to `SyntaxNode`s to be more consistent with IR documents. - Updated test files. In some cases new errors were found because we're now lifting them to the `SyntaxTree`, in most others the errors are re-ordered. **Note: In end-to-end scenarios diagnostics were not hidden, only unordered. The IR phase would find nested/hidden documents and lift them to the IR/C# documents.** \n\nCommit migrated from dotnet/razor@ef31a96
- This adds support for C# single line variations of: `if`, `for`, `foreach`, `do`, `while`, `lock` and `using`. - Turns out the existing parser had 99% of the support for these scenarios already. Therefore, in this change set I went ahead and added exhaustive tests to verify things worked end-to-end. - Added a restriction to single line markup to not allow markup in single line control flow statements. Updated resx to provide a better error message for cases when users do use markup in single line control flow statements. dotnet/razor#9637 \n\nCommit migrated from dotnet/razor@cf474c9
- This is a pre-requisite to properly warnings users about TagHelpers in code blocks. With this change VS indicates that there's an asynchronous operation happening in a code block but because the code is auto-generated it happens behind the scenes and therefore always gets logged as a line 1 error (unusable). Therefore, to fix this issue there will be a partner PR that will go out in AspNetCore that adds analyzer errors for cases where `__tagHelperRunner.RunAsync` exists in code blocks that do not return `Task` and are not marked with `async` properly. - Updated test files to reflect the new TagHelper codegen at design time. - Note: This doesn't solve the `~/` case that will be in a follow up PR. aspnet/AspNetCoredotnet/aspnetcore-tooling#8630 \n\nCommit migrated from dotnet/razor@17af3ef
* Do build time discovery of MVC ApplicationParts \n\nCommit migrated from dotnet/razor@e79d560
… `$(GetCopyToOutputDirectoryItemsDependsOn)` instead of `$(GetCopyToOutputDirectoryItems)`) (dotnet/razor#603) The intention here would have been to add to the existing GetCopyToOutputDirectoryItemsDependsOn property, defined in Microsoft.Common.Targets. Instead what was happening is that it was being completely overwitten with the _RazorGetCopyToOutputDirectoryItems entry, given that $(GetCopyToOutputDirectoryItems) doesn't exist as a property and must have been a typo (it exists as a task, not a property). An effect of this bug was that the `AssignTargetPaths` target is not always executed before the `GetCopyToOutputDirectoryItems` target, which should explicitly depend on it. `AssignTargetPaths` is responsible for populating the `ContentWithTargetPath` itemgroup. Left empty, it appears to `GetCopyToOutputDirectoryItems` task that there are no Content files to copy to the final output directory, when in fact there are.\n\nCommit migrated from dotnet/razor@4597bc3
* Move the globs from the project system into the Razor SDK on a separate file that can be imported by itself. * Import the file conditionally based on the EnableRazorContent property\n\nCommit migrated from dotnet/razor@62e33da
…zor#580) * Imports static assets from packages containing custom msbuild targets defining a StaticWebAsset item group. * Generates an embeds a manifest into the application assembly that contains a list of paths to the content roots of the assets defined in the packages custom msbuild targets.\n\nCommit migrated from dotnet/razor@8e70013
…otnet/razor#597) * Directive attributes part 1: Support parameters in bound attributes * update * Fix test * feedback * Updated design * Do case sensitive comparison * more * Bug fix \n\nCommit migrated from dotnet/razor@9bbf240
\n\nCommit migrated from dotnet/razor@67b8a83
This came up in the context of discussion around aspnet/AspNetCoredotnet/aspnetcore-tooling#5071. Browsers use a 'first attribute wins' rule for evaluating attributes and building the DOM - though duplicate attributes are considered to be a spec violation. Blazor wants to use 'last attribute wins' rules because that's most sensible when you consider evaluation order and splatting. This means that we have to have the markup block pass do the same thing as the Blazor runtime when duplicates appear. However, since this is a spec violation I'm proposing that we just make it an error (like we do for malformed tags). There's no useful purpose for duplicate attributes since browsers ignore it. Since we're not going to allow this for elements. Let's go all of the way and block it for components as well. And while we're here, lets block some of the invalid cases where users try to mix onchange and bind. These don't work today, so we should make them an error. If we decide to add support for this case to the runtime that would require compiler work anyway. \n\nCommit migrated from dotnet/razor@0c59ca5
\n\nCommit migrated from dotnet/razor@5bcd008
* Specify UpToDateReloadFileTypes Specify a list of file extensions that VS will monitor for file changes to reload the app. Includes .razor and .cshtml files. \n\nCommit migrated from dotnet/razor@9a96412
…et/razor#605) * Define static web assets in the current project * These are assets under the wwwroot folder. * By convention, the base path for these assets is `_content/<<PackageId>>` with spaces and dots removed and all characters lower-cased. * Retrieve static web assets from referenced projects\n\nCommit migrated from dotnet/razor@50646aa
* Support parsing directive attributes \n\nCommit migrated from dotnet/razor@5eb8ef4
\n\nCommit migrated from dotnet/razor@986c224
\n\nCommit migrated from dotnet/razor@a7d1373
* Adds support for publishing static web assets * At publish time, it copies all the referenced assets from referenced projects and packages into their final locations. * Automatically pack static web assets for consumption * Generate Microsoft.AspNetCore.StaticWebAssets.props and pack it into build\Microsoft.AspNetCore.StaticWebAssets.props * Generate `<<PackageId>>.props` and pack it into `build\<<PackageId>>.props` importing Microsoft.AspNetCore.StaticWebAssets.props * Generate `<<PackageId>>.props` and pack it into buildMultiTargeting\`<<PackageId>>.props` importing `build\<<PackageId>>.props` * Generate `<<PackageId>>.props` and pack it into buildTransitive\`<<PackageId>>.props` importing buildMultiTargeting\`<<PackageId>>.props` * Pack all the static web assets from the current project into `staticwebassets\**`\n\nCommit migrated from dotnet/razor@87817be
\n\nCommit migrated from dotnet/razor@81c2e03
\n\nCommit migrated from dotnet/razor@42d3a88
…r#639) * Allow UpToDateReloadFileTypes to be modified by packages * Allow file watching with dotnet-watch \n\nCommit migrated from dotnet/razor@fbcd0c2
…zor#638) * Use an explicit intermediate node for directive attributes * More cleanup \n\nCommit migrated from dotnet/razor@f8d7f4c
- WTE needs a way to detect if a `TagHelperDescriptor` is a component based `TagHelper` in order to turn features like validation, completion etc. on/off. - Updated tests to add verification points for the new `IsRazorComponentTagHelper` method. \n\nCommit migrated from dotnet/razor@6fcd12e
…ifest * Stops embedding the manifest in favor of using a file copied to the output folder. \n\nCommit migrated from dotnet/razor@4c65b19
JunTaoLuo
force-pushed
the
johluo/tooling-consolidation-fixup
branch
3 times, most recently
from
c8daeab
to
e12fec5
Compare
pranavkm
approved these changes
May 26, 2020
| <PackageReference Include="Microsoft.NET.Sdk.Razor" PrivateAssets="All" Version="$(MicrosoftNETSdkRazorPackageVersion)" IsImplicitlyDefined="true" /> | ||
| </ItemGroup> | ||
| <Choose> | ||
| <When Condition="'$(UsingMicrosoftNETSdkWeb)' == 'true' OR '$(RazorSdkCurrentVersionProps)' != ''"> |
|
@JunTaoLuo I'll likely look at this tomorrow, when I'm not on-call anymore. |
3 tasks
|
@dougbu I'd like to get this in asap since I need to also react to it in aspnetcore-tooling and I'll need to complete those before preview6 branching. If the build passes I'll likely just merge it and address remaining comments you have separately. |
JunTaoLuo
force-pushed
the
johluo/tooling-consolidation-fixup
branch
from
c0e028e
to
50f3a16
Compare
JunTaoLuo
changed the base branch from
johluo/tooling-soncolidation-source-2
to
master
Not sure I get how 1 day will mess this up. Could you clarify? In addition, merging the two branches made the changes impossible to view in GitHub. How would I verify the changes you made in response to my comments? |
|
I'm trying to get this done as soon as possible. I don't want to jeopardize this work given that there's reactionary changes in aspnetcore-tooling and a mop-up merge. To review, you'd want to take a look at the individual commit. 50f3a16. I've squashed already, so you'll need to review the entire commit again. |
|
Note that the CI has been terribly slow so any change takes a few hours to verify so that's why I want to get things merged. I feel waiting even a day is risky. |
JunTaoLuo
dismissed
dougbu’s stale review
Any lingering comments will be addressed in a mop up PR.
dougbu
reviewed
May 27, 2020
| @@ -149,7 +149,7 @@ stages: | |||
| $(_InternalRuntimeDownloadArgs) | |||
| displayName: Build x86 | |||
|
|
|||
| # This is in a separate build step with -forceCoreMsbuild to workaround MAX_PATH limitations - https://github.com/Microsoft/msbuild/issues/53 | |||
| # This is in a separate build step with to workaround MAX_PATH limitations - https://github.com/Microsoft/msbuild/issues/53 | |||
src/Razor/test/Directory.Build.props
Outdated
| <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.props))\Directory.Build.props" /> | ||
|
|
||
| <PropertyGroup> | ||
| <DisablePubternalApiCheck>true</DisablePubternalApiCheck> |
There was a problem hiding this comment.
Actually, this shouldn't be necessary at all because the $(DisablePubternalApiCheck) property is only read in the root Directory.Build.props and that check also excludes test projects.
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| <PropertyGroup> | |||
| <TargetFrameworks>netstandard2.0;net461</TargetFrameworks> | |||
| <DisablePubternalApiCheck>true</DisablePubternalApiCheck> | |||
There was a problem hiding this comment.
Again, this change shouldn't be necessary given how the root Directory.Build.props uses this property.
src/Razor/Microsoft.NET.Sdk.Razor/src/Microsoft.NET.Sdk.Razor.csproj
Outdated
Show resolved
Hide resolved
|
|
||
| <PropertyGroup> | ||
| <RazorSdkDirectoryRoot>${ArtifactsBinDir}Microsoft.NET.Sdk.Razor\${Configuration}\sdk-output\</RazorSdkDirectoryRoot> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
The logic in eng/targets/CSharp.Common.props also uses $(Configuration) as an input. Why must this new <Import/> and <PropertyGroup/> be repeated six times? The logic will need to change and having it spread out like this will cause problems in the future.
This was referenced May 27, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #21145.
The source code was migrated in the https://github.com/dotnet/aspnetcore/tree/johluo/tooling-soncolidation-source-2 branch and this branch contains the fixes to build and test the new migrated razor packages.