Write out JS comments when there would be 4 or more consecutive tildes in generated Html #10805
Conversation
There was a problem hiding this comment.
The contents, and indeed the expected state of this file, are not important. What is important is that merely opening up the file without the changes in this PR instantly crashes the JS compiler and breaks the editor.
There was a problem hiding this comment.
Perhaps add a comment mentioning that (e.g., linking the issue) so we know what's the purpose of the test in the future.
|
Fun fact: If you stare at enough JS comments in diffs they start to look like worried little guys |
There was a problem hiding this comment.
Perhaps add a comment mentioning that (e.g., linking the issue) so we know what's the purpose of the test in the future.
|
Correct. This is a little piece of tooling that lives in the compiler, but even with FUSE, it shouldn't run during actual builds. |
…ing engine has known issues
|
Fixed one integration test (lucked into an unstable formatting result), the other two failing tests are also failing in main so shouldn't be blocking. UPDATE: Failing integration tests fixed in #10807 |
|
@dotnet/razor-compiler for second review thanks |
|
Anyone on @dotnet/razor-tooling want to share an opinion? |
Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2222322
Our use of tildes as replacement characters for C# in Html documents has never been great, and has caused issues with JS/TS and Html tooling in the past, but it seems there are scenarios where large/complicated/specifically formed Razor documents can actually crash the JS compiler. Seems it has a perf issue where lots of unary negation (ie,
~) causes stack size issues.This PR mitigates the issue by encoding any stretch of C# characters that are 4 characters or longer as a JS comment (
/*~~~*/) so that the compiler will ignore some of the more problematic chunks of Razor files. There is one case where we couldn't do that, but any pressure relief should help even if it's not 100%.