fix(error): 404 redirect when clicking load more on account stream #296
Conversation
|
Good catch! Indeed, I recently discovered that the prettier-based auto-formatting that I had added a few months back can cause mustache/hogan templates to break, when they are embedded in JavaScript code, because they are mistakingly taken as JavaScript objects by the formatter. 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. Do you have any other idea on how to prevent this kind of template-breaking formatting from happening again? |
|
@compiuta also, about:
Did you mean that you can't test locally because you have not enough tracks locally to be able to paginate? In that case, this scipt may help: https://github.com/openwhyd/openwhyd/blob/master/docs/INSTALL.md#sample-data :-) EDIT: I'm about to write a regression test for the "load more" pagination, so that we can detect if it breaks again in the future. |
|
@adrienjoly it looks like adding comments around blocks of code is the correct way to prevent prettier formatting https://prettier.io/docs/en/ignore.html I have added multiline comments around template conditions inside the script tags and the error is no longer showing. |
|
Great job, Compiuta!! 👏 Thanks for doing that! I'm almost done on the non-regression test. My plan is to merge it to master, to show that the build is broken. And then to merge your PR (that should hopefully fix the build). |
In PR #296, we found out that a reformat of our front-end js files including mustache/hogan blocks caused runtime errors which prevented the "load more" button to work as expected. In order to prevent future regressions on pagination and runtime errors, this PR adds Cypress tests. Note: It's expected that the Cypress tests fails, until we merge #296.
|
My PR (with failling non-regression test) is done and merged to I'm going to update your PR to the latest version of that branch. => The test should hopefully pass thanks to your fix. |
|
Thank you, @compiuta, you rock! 💪 |
* master: (64 commits) chore(release): 1.30.6 [skip ci] fix(ui): Resizing the page should not scroll to the top (#303) chore(release): 1.30.5 [skip ci] fix(tests): Migrate acceptance tests from Webdriver to Cypress (#301) chore(release): 1.30.4 [skip ci] fix(tests): create dummy posts from cypress tests instead of from initdb_testing (#304) chore(release): 1.30.3 [skip ci] fix(tests): Reset database between each Cypress tests (#302) chore(release): 1.30.2 [skip ci] fix(ui): Display correct track error message when using electron app (#294) chore(release): 1.30.1 [skip ci] fix(error): 404 redirect when clicking load more on account stream (#296) test(#295): add non-regression test for pagination (#299) chore(release): 1.30.0 [skip ci] feat(ci): separate ci tasks for each type of tests (#298) chore(release): 1.29.0 [skip ci] feat(ci): enable TypeScript in Cypress tests (#297) remove browser cache hack preventing css from displaying correct logo (#293) chore(release): 1.28.2 [skip ci] fix(ui): never-ending loading animation on empty search (#260) ... # Conflicts: # .github/workflows/nodejs.yml
Closes #295 .
What does this PR do / solve?
It looks like the user gets redirected to the 404 page because of a javascript error that involves the template condition inside the script tag. This error prevents the function from running and rendering the correct url for the button.
Overview of changes
I added individual script tags for each template condition. This removed the error showing up in the console.
How to test this PR?
@adrienjoly I am having the same issue with data not being able to load when testing locally on my machine so I am not able to test this solution. Let me know if you are able to test this.