Skip to content
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

Fix backtick string literals not being recognized in the svg/tag/props properties #1285

Merged

Conversation

TehShrike
Copy link
Member

Failing test for #1284
Fixes #1284

@codecov-io
Copy link

codecov-io commented Mar 28, 2018

Codecov Report

Merging #1285 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1285      +/-   ##
==========================================
+ Coverage   91.55%   91.62%   +0.06%     
==========================================
  Files         121      122       +1     
  Lines        4324     4333       +9     
  Branches     1362     1368       +6     
==========================================
+ Hits         3959     3970      +11     
+ Misses        153      150       -3     
- Partials      212      213       +1
Impacted Files Coverage Δ
src/utils/nodeToString.ts 100% <100%> (ø)
src/generators/Generator.ts 93.93% <100%> (ø) ⬆️
src/validate/js/propValidators/namespace.ts 100% <100%> (ø) ⬆️
src/validate/js/index.ts 82.85% <100%> (ø) ⬆️
src/validate/js/propValidators/tag.ts 100% <0%> (ø) ⬆️
src/validate/js/propValidators/props.ts 57.14% <0%> (+57.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b506e5a...864fd31. Read the comment docs.

@Conduitry
Copy link
Member

I think we'd want to make sure that this is a template string with no actual ${template} things in it. In general that seems like a good thing to check for and to support wherever we're currently requiring string literals.

@TehShrike
Copy link
Member Author

Aye, that's what the prop.value.quasis.length === 1 && prop.value.expressions.length === 0 is checking for.

@Conduitry
Copy link
Member

Ah, whoops! Missed that nodeToString would return undefined if that weren't the case, and then its caller would check whether that was a string.

But I do think that it'd be good to also use the same logic here and here.

@TehShrike
Copy link
Member Author

Want that done in this PR?

@Conduitry
Copy link
Member

Yes please I think that'd be good

@TehShrike TehShrike changed the title Fix backtick string literals not being recognized for the svg property Fix backtick string literals not being recognized in the svg/tag/props properties Mar 28, 2018
@Conduitry Conduitry merged commit 565bc52 into sveltejs:master Mar 29, 2018
TehShrike added a commit to KayserCommentaryOrg/crucifixion-week that referenced this pull request Mar 29, 2018
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.

3 participants