Skip to content

Code dump of component tests#47

Merged
rynowak merged 1 commit intomasterfrom
rynowak/component-test-dump
Dec 18, 2018
Merged

Code dump of component tests#47
rynowak merged 1 commit intomasterfrom
rynowak/component-test-dump

Conversation

@rynowak
Copy link
Member

@rynowak rynowak commented Dec 18, 2018

This is a dump of the existing component codegen tests from
the Blazor effort.

I'm also adding the minimal Component types as a new shim to get things
to compile. This is similar to how our shims work already for MVC.

The next step will be add port functionality until the tests pass.

Summary of the changes (Less than 80 chars)

  • Detail 1
  • Detail 2

Addresses #bugnumber (in this specific format)

This is a dump of the existing component codegen tests from
the Blazor effort.

I'm also adding the minimal Component types as a new shim to get things
to compile. This is similar to how our shims work already for MVC.

The next step will be add port functionality until the tests pass.
@rynowak
Copy link
Member Author

rynowak commented Dec 18, 2018

I plan to merge this when the CI passes since this is just a dump of things we have elsewhere. The subsequent PRs will do the actual integration work.

@rynowak rynowak merged commit 49f49dd into master Dec 18, 2018
@rynowak rynowak deleted the rynowak/component-test-dump branch December 18, 2018 02:21
NTaylorMullen pushed a commit that referenced this pull request Sep 10, 2019
- This is handled by the in-the-box HTML language server support for .cshtml documents. By removing our understanding of HTML we'll get JavaScript project awareness for free.
- Updated functional tests to reflect new behavior.

#47
chsienki pushed a commit to chsienki/razor-tooling that referenced this pull request Aug 26, 2022
…0.2xx-to-main

[automated] Merge branch 'release/6.0.2xx' => 'main'
DustinCampbell added a commit to DustinCampbell/razor that referenced this pull request Sep 4, 2025
This change refactors ComponentMarkupEncodingPass.TryGetHtmlEntity(...) to avoid allocations by using spans where possible instead of creating sub-strings. However, while doing this I found some very strange behavior.

Numeric HTML character entities references allow both decimal and hexadecimal numbers in the form of `&#nnnn;` and `&#xhhhh;`, respectively. However, it seems that Razor's decoding never supported hexadecimal numbers properly. The original implementation uses `Convert.ToInt32(..., 16)` to parse hexadecimal numbers, which supports leading '0x' but not a leading 'x'. So, Razor *only* support an *illegal* variant of hex character entities: `&#0xhhhh;`. Surprisingly, there's even a test (Execute_MixedHtmlContent_MultipleHTMLEntities_DoesNotSetEncoded) that validates an illegal hex character entity!

In addition, I found that the Razor compiler will fail to decode numeric character entities that do not appear in the ParserHelpers.HtmlEntityCodePoints dictionary. This is strange because the dictionary is far from complete! There are many basic character entities (like '&dotnet#47;' -> '/') that are not present.

Character entity decoding in the compiler is really a best effort. If it fails for any reason, the compiler will generate a call to RenderTreeBuilder.AddMarkupContent with the encoded text instead of RenderTreeBuilder.AddContent. However, when that happens, it means that character decoding will happen at runtime, which might(?) have performance implications.

I've gone ahead and fixed both of these issues:

1. Added support to TryGetHtmlEntity for legal hex character entities in addition to the illegal variants.
2. Added support to TryGetHtmlEntity for legal printable code points that don't happen to appear in the ParserHelpers.HtmlEntityCodePoints dictionary.
DustinCampbell added a commit that referenced this pull request Sep 8, 2025
> [!TIP]
> I strongly recommend reviewing this commit-by-commit. The commit
history represents a progression of changes, and each commit has a
detailed message describing its purpose.

This change overhauls the HTML character entity reference decoding
performed by `ComponentMarkupEncodingPass`. The Razor compiler makes a
best effort to decode HTML content for code generation. If it determines
that the content is suitable as raw text and all character entity
references can be decoded (if any), the compiler's code gen will call
`RenderTreeBuilder.AddContent(...)`. If not, it'll call
`RenderTreeBuilder.AddMarkupContent(...)`, which can have an impact on
the runtime performance of the compiled application. So, the goal is to
call `AddContent(...)` as much as possible.

While tuning the performance of `ComponentMarkupEncodingPass` to reduce
allocations and avoid unnecessary work, I encountered two bugs:

1. The Razor compiler doesn't implement hexadecimal HTML character
entity references correctly. Hex character entity references are
supposed to be in the form, `&#xhhhh;`. However, the compiler's
implementation only supports an illegal form, `&#0xhhhh;` and doesn't
actually support the legal form. So, if content contains legal hex
numeric character entity references, they will fail to decode and
`RenderTreeBuilder.AddMarkupContent(...)` will be code-gen'd.
2. When decoding a decimal character entity reference, such as `&#1234`,
a lookup table is used to retrieve the code point value. However, the
lookup table isn't complete and doesn't include very simple values. For
example, it doesn't include `/` which translates to a `/` character.
So, if the content contains decimal numeric character entity references
that aren't included in the lookup table, they will fail to decode and
`RenderTreeBuilder.AddMarkupContent(...)` will be code-gen'd.

I've gone ahead and fixed both of these issues in
6f23ef0.

In addition, there are several performance and allocation fixes in this
commit. These changes resulted in the creation of a handful of useful
helpers for building and creating strings using a
`MemoryBuilder<ReadOnlyMemory<char>>`. Those helpers and tests are
included in e266160.

Many thanks to Copilot for help with writing all of the tests. 🤖🧪❤️

----
CI Build:
https://dev.azure.com/dnceng/internal/_build/results?buildId=2786521&view=results
Test Insertion:
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/667364
Toolset Run:
https://dev.azure.com/dnceng/internal/_build/results?buildId=2786524&view=results
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.

1 participant