Added the ability to exclude frontend reload script injection via configuration #1457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alallier
commented
Aug 28, 2024
| @@ -163,6 +163,35 @@ describe('frontend reload', () => { | |||
| assert(res.text.includes('<script src=\'/reloadHttp/reload.js\'></script>')) | |||
| }) | |||
|
|
|||
| it('should exclude multiple glob paths for auto inject reload script into rendered html', async () => { | |||
There was a problem hiding this comment.
This wording isn't great...
alallier
force-pushed
the
reloadConfig
branch
2 times, most recently
from
f9ea044
to
ee51f01
Compare
alallier
commented
Aug 28, 2024
| } | ||
| }) | ||
|
|
||
| // check that reload script is injected into response bodies |
There was a problem hiding this comment.
Copy/paste comment is no longer correct
alallier
commented
Aug 28, 2024
|
|
||
| router.route('/HTMLTest/nested').get((req, res) => { | ||
| // save the path of the plain HTML page | ||
| const htmlPath = path.join(__dirname, '../views/plainHTMLTest.html') |
There was a problem hiding this comment.
To make these more re-useable later and semantically correct we might want new .html documents.
Let me know and I can update
…b patterns at the server configuration level. * Added minimatch as a dependency * Added new tests to test reload excludeRoutes * Added default excludeRoutes as empty array
|
We need to hold merge on this for a bit, the test was working before but now it's breaking and there are recursion depth problems that could cause performance issues. Need to handle both of those items before merging |
kethinov
requested changes
Aug 29, 2024
| } | ||
| ``` | ||
|
|
||
| - `excludeRoutes`: Reload auto script injection happens on all HTML routes by default unless specified in `excludeRoutes`. An array of valid glob patterns is accepted |
There was a problem hiding this comment.
Suggested change
| - `excludeRoutes`: Reload auto script injection happens on all HTML routes by default unless specified in `excludeRoutes`. An array of valid glob patterns is accepted | |
| - `excludeRoutes`: List of routes to exclude from Reload automatically injecting its script onto. | |
| - Default: *[Array]* with no items in it. This means Reload will inject its script on all routes by default. |
|
|
Autre31415
reviewed
Aug 29, 2024
| @@ -372,10 +372,13 @@ Resolves to: | |||
| { | |||
| "enable": true, | |||
| "port": 9856, | |||
| "httpsPort": 9857 | |||
| "httpsPort": 9857, | |||
| "excludeRoutes": [] | |||
There was a problem hiding this comment.
The HTML minifier has a similar param called exceptionRoutes, either this new one, or the old one should be renamed for consistency.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the ability to exclude frontend reload script injection via glob patterns at the server configuration level.
(Closes #1150)