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

Protect mustache/hogan blocks from being altered by prettier => prevent runtime errors #300

Closed
adrienjoly opened this issue Mar 21, 2020 · 1 comment · Fixed by #345
Closed

Comments

@adrienjoly
Copy link
Member

adrienjoly commented Mar 21, 2020

In #296, we found that, while reformatting a HTML template that contained mustache/hogan blocks embedded in JavaScript code, prettier broke those blocks, causing bugs in the UI.

For instance, resulting to the following invalid template:

{ { # shouldDisplay } }
window.alert('coucou');
{ { / shouldDisplay } } 

The best workaround I found so far was to add multiline JS comments around those templating elements.

For instance:

/*{{#shouldDisplay}}*/
window.alert('coucou');
/*{{/shouldDisplay}}*/

=> This workaround prevents prettier from trying to re-format these templating elements.

Let's prevent these kinds of bugs from happening again by protecting these blocks.

Note: we're open for alternative ideas on how to fix this reliably and sustainbly.

@adrienjoly adrienjoly added this to 📥 Inbox / ideas in Development via automation Mar 21, 2020
@adrienjoly adrienjoly moved this from 📥 Inbox / ideas to ⚡️To Do Next in Development Mar 21, 2020
@adrienjoly adrienjoly moved this from ⚡️To Do Next to 📥 Inbox / ideas in Development Aug 25, 2020
Development automation moved this from 📥 Inbox / ideas to ✔️ Done / pending QA Aug 27, 2020
@adrienjoly adrienjoly moved this from ✔️ Done / pending QA to 🌲 In production in Development Aug 29, 2020
@adrienjoly adrienjoly moved this from 🌲 In production to ✔️ Done / pending QA in Development Aug 29, 2020
adrienjoly added a commit that referenced this issue Aug 29, 2020
…iles (#344)

Fixes 1025 issues reported by Codacy. 💪

We don't let Prettier reformat HTML files yet, because of #300.
@adrienjoly adrienjoly moved this from ✔️ Done / pending QA to ⚙ In progress in Development Aug 29, 2020
@adrienjoly adrienjoly reopened this Aug 29, 2020
adrienjoly pushed a commit that referenced this issue Aug 29, 2020
## [1.34.13](v1.34.12...v1.34.13) (2020-08-29)

### Bug Fixes

* **clean-up:** Setup linter and formatting => apply to (almost) all files ([#344](#344)) ([224def8](224def8)), closes [#300](#300)
Development automation moved this from ⚙ In progress to ✔️ Done / pending QA Aug 29, 2020
adrienjoly pushed a commit that referenced this issue Aug 29, 2020
## [1.34.14](v1.34.13...v1.34.14) (2020-08-29)

### Bug Fixes

* **clean-up:** `npm run lint:fix` to also format HTML files ([#345](#345)) ([950d981](950d981)), closes [#300](#300)
@adrienjoly adrienjoly moved this from ✔️ Done / pending QA to 🌲 In production in Development Aug 29, 2020
@adrienjoly
Copy link
Member Author

Spotted { {#hasMore } } in app/templates/userProfileV2.html, after letting vscode reformat the js/jquery section of the file, on save, while developing PR #346:

image

@adrienjoly adrienjoly reopened this Aug 29, 2020
Development automation moved this from 🌲 In production to ⚙ In progress Aug 29, 2020
adrienjoly added a commit that referenced this issue Aug 29, 2020
(note: found issue #300 while formating with vscode)
adrienjoly added a commit that referenced this issue Aug 29, 2020
adrienjoly added a commit that referenced this issue Aug 30, 2020
## Problem

Reported by Robert S.:

> Hello,  I was transferring a bunch of tracks from Playmoss to Openwhyd this am, as that site I believe is shutting down after this weekend.  Then, my Open Whyd + Track extension stopped connecting.  I reinstalled it, still the same issue.  Is there a limit set to how many tracks a user can add to a playlist in one day?

## How to reproduce

On Openwhyd v1.34.14 (4294c67)

1. go to any YouTube video page
2. activate the bookmarklet (on your local js console, type: `window.document.body.appendChild(window.document.createElement('script')).src = 'http://localhost:8080/js/bookmarklet.js';`)
3. click on the first thumb => it will open `/post?v=2&embed=1&eId=%2Fyt%2FGwL-COqlauA...` in a dialog/popup window
4. the dialog hangs, with the following error in the JS console: `Uncaught SyntaxError: Unexpected token ','`

## Solution

This PR fixes the `Uncaught SyntaxError: Unexpected token ','` error happening when trying to add a track, on `/post?v=2&embed=1&eId=%2Fyt%2FGwL-COqlauA`, by fixing a syntax error caused by an automatic formatting of `postEditV2.html` which added a comma (`,`) after a closing Mustache template. Related to #300. See the `fix` commit in the PR.

It also tries to reduce the risk of a "Uncaught ReferenceError: YOUTUBE_API_KEY is not defined" error happening while running bookmarklet test.
Development automation moved this from ⚙ In progress to ✔️ Done / pending QA Aug 30, 2020
@adrienjoly adrienjoly moved this from ✔️ Done / pending QA to 🌲 In production in Development Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development
  
🌲 In production
Development

Successfully merging a pull request may close this issue.

1 participant