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

Usability improvement: Block script tags in components, giving explanation #553

Merged
merged 1 commit into from
Apr 12, 2018

Conversation

SteveSandersonMS
Copy link
Member

Implementation for #552

@SteveSandersonMS
Copy link
Member Author

STILL TO DO: Replace the fake link in the error message with a real fwlink.

@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/prevent-script-tags branch from a90638e to d1bfdfe Compare April 12, 2018 10:02
@SteveSandersonMS
Copy link
Member Author

I propose we include this in 0.2.0.

{
var adjustedSpan = CalculateSourcePosition(node.Source, tagToken.Position);
var diagnostic = BlazorDiagnosticFactory.Create_DisallowedScriptTag(adjustedSpan);
throw new RazorCompilerException(diagnostic);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine for now, but keep in mind that any time you throw and catch in the compiler, that means that someone hitting the error will lose all intellisense and colorization in the rest of the file after this point. If at all possible, we should just skip the offending tag and its contents.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the case for code in the runtime node writer as well as the design-time one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with the runtime writer is that you won't run through the rest of the file, so if you throw here it will suppress any other errors in the file.

I don't have a problem with doing this as a temporary fix. I have a todo to remove all of these and the exception.

@sjroe
Copy link
Contributor

sjroe commented Apr 12, 2018

Note that (as expected) this doesn't prevent a script tag from being used when writing a component directly without Razor.

@SteveSandersonMS SteveSandersonMS deleted the stevesa/prevent-script-tags branch April 30, 2018 08:50
SteveSandersonMS added a commit to SteveSandersonMS/BlazorMigration that referenced this pull request Nov 27, 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.

3 participants