10000 [AssetMapper] JavaScriptImportPathCompiler::compile(): Return value must be of type string, null returned · Issue #54078 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[AssetMapper] JavaScriptImportPathCompiler::compile(): Return value must be of type string, null returned #54078

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.

Sign up for GitHub

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

Jump to bottom
Closed
PhilETaylor opened this issue Feb 27, 2024 · 11 comments
Closed

[AssetMapper] JavaScriptImportPathCompiler::compile(): Return value must be of type string, null returned #54078

PhilETaylor opened this issue Feb 27, 2024 · 11 comments
Labels
AssetMapper Bug Status: Needs Review

Comments

@PhilETaylor
Copy link
Contributor
PhilETaylor commented Feb 27, 2024 •
edited
Loading

Symfony version(s) affected

6.4.4

Description

Regression in AssetMapper 6.4.4 released today

ScreenShot-2024-02-27-08 57 24

How to reproduce

symfony new . --version="6.4.*" --webapp
php bin/console importmap:req @yaireo/tagify
php bin/console importmap:update

ScreenShot-2024-02-27-09 20 40

Possible Solution

No response

Additional Context

No response
@PhilETaylor
Copy link
Contributor Author

A quick test with a preg_match at the start of the compile method like

ScreenShot-2024-02-27-09 08 28

gives

ScreenShot-2024-02-27-09 08 00

@Chris53897
Copy link
Contributor
Chris53897 commented Feb 27, 2024 •
edited
Loading

Same for Symfony 7.0.4
--env=prod asset-map:compile -q

09:14:30 CRITICAL  [console] Error thrown while running command "--env=prod asset-map:compile -q". Message: "Symfony\Component\AssetMapper\Compiler\JavaScriptImportPathCompiler::compile(): Return value must be of type string, null returned" ["exception" => TypeError { …},"command" => "--env=prod asset-map:compile -q","message" => "Symfony\Component\AssetMapper\Compiler\JavaScriptImportPathCompiler::compile(): Return value must be of type string, null returned"]

In JavaScriptImportPathCompiler.php line 119:
                                                                                                                                      
  Symfony\Component\AssetMapper\Compiler\JavaScriptImportPathCompiler::compile(): Return value must be of type string, null returned

@smnandre Could you please check if this is related?
#53652

@fracsi
Copy link
Contributor
fracsi commented Feb 27, 2024 •
edited
Loading

The preg_last_error_msg is JIT stack limit exhausted
Quick fix: disable pcre.jit in php.ini

@nicolas-grekas
Copy link
Member

Can you try this diff? Does that help?

--- a/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php
+++ b/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php
@@ -31,13 +31,12 @@ final class JavaScriptImportPathCompiler implements AssetCompilerInterface
      * @see https://regex101.com/r/1iBAIb/1
      */
     private const IMPORT_PATTERN = '/
-        ^
-            (?:\/\/.*)                     # Lines that start with comments
+            ^(?:\/\/.*)                     # Lines that start with comments
         |
             (?:
-                \'(?:[^\'\\\\\n]|\\\\.)*\'   # Strings enclosed in single quotes
+                \'(?:[^\'\\\\\n]|\\\\.)*+\'   # Strings enclosed in single quotes
             |
-                "(?:[^"\\\\\n]|\\\\.)*"      # Strings enclosed in double quotes
+                "(?:[^"\\\\\n]|\\\\.)*+"      # Strings enclosed in double quotes
             )
         |
             (?:                            # Import statements (script captured)
@@ -49,7 +48,7 @@ final class JavaScriptImportPathCompiler implements AssetCompilerInterface
             |
                 \bimport\(
             )
-            \s*[\'"`](\.\/[^\'"`\n]+|(\.\.\/)*[^\'"`\n]+)[\'"`]\s*[;\)]
+            \s*[\'"`](\.\/[^\'"`\n]++|(\.\.\/)*+[^\'"`\n]++)[\'"`]\s*[;\)]
         ?
     /mx';

@PhilETaylor
Copy link
Contributor Author
PhilETaylor commented Feb 27, 2024 •
edited
Loading

Hey @nicolas-grekas YES! I apply (manually) your diff and now the import is correctly made with no errors.

private const IMPORT_PATTERN = '/
        ^
           ^(?:\/\/.*)                     # Lines that start with comments
        |
            (?:
                \'(?:[^\'\\\\\n]|\\\\.)*+\'   # Strings enclosed in single quotes
            |
                "(?:[^"\\\\\n]|\\\\.)*+"       # Strings enclosed in double quotes
            )
        |
            (?:                            # Import statements (script captured)
                import\s*
                    (?:
                        (?:\*\s*as\s+\w+|\s+[\w\s{},*]+)
                        \s*from\s*
                    )?
            |
                \bimport\(
            )
             \s*[\'"`](\.\/[^\'"`\n]++|(\.\.\/)*+[^\'"`\n]++)[\'"`]\s*[;\)]
        ?
    /mx';

ScreenShot-2024-02-27-09 47 56

@nicolas-grekas
Copy link
Member

Cool. Up for a PR?

@PhilETaylor PhilETaylor mentioned this issue Feb 27, 2024
Merged
@PhilETaylor
Copy link
Contributor Author

#54079

@Chris53897
Copy link
Contributor

Thanks for the Fix.
Should this Issue maybe reopened, just to Inform other users until 7.0.5 is released?

@PhilETaylor PhilETaylor reopened this Feb 27, 2024
@ThomasLandauer
Copy link
Contributor

Other quick fix: Add this to your composer.json to downgrade to 7.0.3:

"conflict": {
    "symfony/asset-mapper": "7.0.4"
},

@stof
Copy link
Member
stof commented Feb 27, 2024

@Chris53897 no. Open issues are the unresolved.

This was referenced Feb 27, 2024
Closed
Closed
@fracsi fracsi mentioned this issue Feb 28, 2024
Closed
@xabbuh
Copy link
Member
xabbuh commented Feb 28, 2024

reopening as the linked PR isn't merged yet

@xabbuh xabbuh reopened this Feb 28, 2024
@xabbuh xabbuh mentioned this issue Feb 28, 2024
Closed
nicolas-grekas added a commit that referenced this issue Feb 29, 2024
@nicolas-g C919 rekas
bug #54079 [AssetMapper] Fix JavaScriptImportPathCompiler regressio…
91278da 
…n in regex (PhilETaylor)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Fix `JavaScriptImportPathCompiler` regression in regex

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix #54078
| License       | MIT

Fixes regression in regex, fix provided by `@nicolas`-grekas in #54078

Commits
-------

ae16b2d [AssetMapper] Fix `JavaScriptImportPathCompiler` regression in regex
@smnandre smnandre mentioned this issue Feb 29, 2024
Merged
@stof stof mentioned this issue Mar 1, 2024
Closed
@derrabus derrabus mentioned this issue Mar 3, 2024
Closed
qdequippe added a commit to jobbsy-dev/jobbsy that referenced this issue Mar 4, 2024
@qdequippe
quick fix compile assets
ba14488 
See symfony/symfony#54078
fabpot added a commit that referenced this issue Mar 4, 2024
@fabpot
bug #54113 [AssetMapper] Throw exception in Javascript compiler when …
5f78910 
…PCRE error (smnandre)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Throw exception in Javascript compiler when PCRE error

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #...
| License       | MIT

`preg_match_callback` can return null when a PCRE error occured, leading there to a TypeError.

Let's throw an exception and expose the error message.

(follows #54078 / complementary to #54079 )

Commits
-------

2333b58 [AssetMapper] Throw exception in Javascript compiler when PCRE error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
AssetMapper Bug Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@nicolas-grekas @PhilETaylor @stof @fracsi @ThomasLandauer @xabbuh @Chris53897 @carsonbot
0