fix(editor): Replace more variants of BASE_PATH in static assets #9564
Conversation
| @@ -141,6 +141,7 @@ export class Start extends BaseCommand { | |||
| createReadStream(filePath, 'utf-8'), | |||
| replaceStream('/{{BASE_PATH}}/', n8nPath, { ignoreCase: false }), | |||
| replaceStream('/%7B%7BBASE_PATH%7D%7D/', n8nPath, { ignoreCase: false }), | |||
| replaceStream('/%257B%257BBASE_PATH%257D%257D/', n8nPath, { ignoreCase: false }), | |||
There was a problem hiding this comment.
Could we add a test to prevent this bug from happening happening ?
There was a problem hiding this comment.
there are no tests for this file. Considering the amount of work required to refactor this file to be able to test it, I'd much rather spend that time fixing the vite config, and remove the two additional replaceStream calls here.
There was a problem hiding this comment.
Would an integration test making sure the font files load make sense?
There was a problem hiding this comment.
by integration I mean e2e*
There was a problem hiding this comment.
since this issue only happens for favicon and fonts, we could update one of the e2e tests to assert that the favicon url was loaded. besides that I can't think of any other reliable way to test this via e2e.
There was a problem hiding this comment.
Would you please try that then if it makes sense?
There was a problem hiding this comment.
Tried to add testing the favicon, but seems not to load with or without the fix, so just let it go for now.
n8n-assistant
bot
added
core
|
Got released with |
Vite upgrade in #9545 has started escaping
{{BASE_PATH}}twice for a couple of places. Until we update vite config to escape these strings more reliably, adding this additional replacement on server startup fixes the issue.Related tickets and issues
https://linear.app/n8n/issue/ADO-2251
#9559
Review / Merge checklist