[AssetMapper] Fix JavaScript compiler load imports from JS strings #53652
Conversation
|
Hey! Thanks for your PR. You are targeting branch "7.1" but it seems your PR description refers to branch "6.4". Cheers! Carsonbot |
| $lineStart = strrpos($fullContent, "\n", $offsetStart - \strlen($fullContent)); | ||
| // Remove enclosed strings before the import | ||
| $lineContentBeforeImport = substr($fullContent, $lineStart, $offsetStart - $lineStart); | ||
| $lineWithoutStrings = preg_replace('/\'(?:[^\'\\\\]|\\\\.)*\'|"(?:[^"\\\\]|\\\\.)*"/', '', $lineContentBeforeImport); |
There was a problem hiding this comment.
template literals (`) are quite common in modern js, and this syntax allows multiline strings making this check quite a challenge
There was a problem hiding this comment.
@kaznovac this PR fixes a bug with console log / deprecation / help messages (cf the original issue)
Handling multi-line strings/comments can reveal itself really complex, and probably would need there some concrete JS parser.
I must admit i doubt any library would use multi-line template comments in source code containing import instructions.. but if/when that happen we could handle it in a later PR ?
WDYT ?
There was a problem hiding this comment.
alternative implementation idea: we merge this regexp with the one that checks for imports.
Since regexp are greedy, this will make the filter in one go, with just a quick check at the start of the closure
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
@nicolas-grekas got it, and it seems to work perfectly ! Thank you :)
There was a problem hiding this comment.
@smnandre great work && sorry for the late answer
p.s. please note that this method can now be removed
src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php
Show resolved
Hide resolved
b8aa830 to
d70e500
Compare
|
Thank you @smnandre. |
Add a check to ensure the import is not parsed from a JS string (e.g.
console.log(...)in the issue)