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

Octal literals and escape sequences are not allowed in template strings (only in development) #766

Closed
1 of 5 tasks
thescientist13 opened this issue Oct 12, 2021 · 2 comments · Fixed by #792
Closed
1 of 5 tasks
Assignees
Labels
bug Something isn't working Plugins Greenwood Plugins question Further information is requested v0.19.1
Milestone

Comments

@thescientist13
Copy link
Member

Type of Change

  • New Feature Request
  • Documentation / Website
  • Improvement / Suggestion
  • Bug
  • Other (please clarify below)

Summary

Not sure if a Greenwood issue per se (that we can fix / handle), but could be a common issue for users, so thought it was worth tracking and to see if anyone has thoughts / contributions towards solving.

So in trying to migrate my studio website from Angular + Bootstrap v4 => Lit + Bootstrap v4, ran into this issue regarding the source code of bootstrap and font-awesome.

Basically they each have code like this

.fa-arrow-left:before {
  content: "\f060";
}

.blockquote-footer::before {
  content: "\2014\00A0";
}

These are octal expressions / literals, and are deprecated and are not allowed in ES6+ / ESM (strict mode), which means when using ESM to import CSS (like with plugin-import-css, the browser throws an exception and stops rendering everything.
Screen Shot 2021-10-11 at 5 45 54 PM
Screen Shot 2021-10-11 at 1 25 00 PM

Details

Curiously, this does not seem to happen when running a production build, and I was able to get everything to style correctly?
Screen Shot 2021-10-12 at 11 36 04 AM

So based on the error which implies es-module-shims is throwing the exception, maybe this is something that can be circumvented? I tried just using the escaping suggestions from the MDN docs in our plugin and that made the error go away BUT then the icons didn't show correctly.
Screen Shot 2021-10-11 at 1 27 35 PM

I also tried not using template literals to wrap the converted CSS but that still left the browser saying this cant be used in strict mode. 🤷‍♂️ 😑
Screen Shot 2021-10-11 at 5 45 54 PM

Additional references / resources

@thescientist13 thescientist13 added bug Something isn't working question Further information is requested CLI labels Oct 12, 2021
@thescientist13 thescientist13 added this to the 1.0 milestone Oct 12, 2021
@thescientist13 thescientist13 self-assigned this Oct 12, 2021
@thescientist13 thescientist13 added Plugins Greenwood Plugins and removed CLI labels Oct 12, 2021
@thescientist13
Copy link
Member Author

Maybe I'm just doing something wrong, because this PR in the Lit docs seems to indicate the method of escaping I was trying should be supported
https://github.com/lit/lit.dev/pull/535/files/bbd9028f39e9602be9d2e01254eb8d14e4fcde7a#diff-a6f64d15022e28ccf904878c816db1f1188236f72c6ffa607ea858689f4ba7ffR115

Maybe it was something else then that was causing the browser to display the escaped HTML instead of the icon? Maybe I needed to use unsafeHTML?

Also, interesting that this only happens during development? Aren't there still template strings in the production build? 🤔 🤔 🤔

@thescientist13
Copy link
Member Author

thescientist13 commented Nov 7, 2021

Ah, so it turned out that the escaping I was doing was indeed correct, but the reason the styles were broken, specifically the font awesome / bootstrap styles was unrelated to this particular issue. Phew.

Screen Shot 2021-11-08 at 10 32 11 AM
Screen Shot 2021-11-08 at 10 32 34 AM

So it seems the main takeaway here is we just need to properly escape \ and test for it using these new real world scenarios in the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Plugins Greenwood Plugins question Further information is requested v0.19.1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant