Repository: symfony/ux (php)

Issue:
[Stimulus Bundle] Exception thrown when `.ts` controller are transpiled to `.js` in the same directory
The [`loadCustomControllers`](https://github.com/symfony/ux/blob/61819ae6c29f5bc6c159c83bf801cef9c0227c82/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php#L64) function iterates over all `.ts` and `.js` files in the project's controllers path to build a controller map.

My project has a build step where TypeScript controllers are transpiled to JavaScript files that are saved alongside the source TypeScript files. My asset mapper config excludes TypeScript files since I'm only interested in serving JavaScript.

When `loadCustomControllers` iterates over all `.ts` and `.js` files in my controllers path, it tries to load mapped assets for my `.ts` controllers. But since these files are excluded, the returned asset is `null` and an exception is thrown when it tries to read the `sourcePath` property on the `null` asset.

Controller files *should* be a mapped asset. Not finding one indicates a problem so an exception *should* be thrown (e.g. see [`null` mapped asset check](https://github.com/symfony/ux/blob/61819ae6c29f5bc6c159c83bf801cef9c0227c82/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php#L134) in [`loadUxControllers`](https://github.com/symfony/ux/blob/61819ae6c29f5bc6c159c83bf801cef9c0227c82/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php#L92) function). However the `loadCustomControllers` function doesn't realize there are `.js` and `.ts` versions of the same controller and can skip processing the missing version.

I expected that I could exclude `.ts` controller files when `.js` versions are available.

Here's my proposed fix:

- Sort matched controller files by filename so `.js` files are given priority over similarly named `.ts` files
- Skip previously mapped controllers with the same name
- Add a null asset check and throw an exception similar to one thrown in `loadUxControllers`

Related: #1210

Pull request:
[StimulusBundle] Skip mapping .ts controller if .js version is available

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Docs?         | no <!-- required for new features -->
| Issues        | Fix #2701  <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

Resolves issue #2701 where an exception is thrown when `.ts` controllers are transpiled to `.js` in the same directory and `.ts` files are excluded from mapped assets. See related issue for details.


Patch:
diff --git a/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php b/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php
index b99f5ffb..9799cdd3 100644
--- a/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php
+++ b/src/StimulusBundle/src/AssetMapper/ControllersMapGenerator.php
@@ -70,6 +70,11 @@ class ControllersMapGenerator
 
         $controllersMap = [];
         foreach ($finder as $file) {
+            // Skip .ts controller if .js version is available
+            if ('ts' === $file->getExtension() && file_exists(substr($file->getRealPath(), 0, -2).'js')) {
+                continue;
+            }
+
             $name = $file->getRelativePathname();
             // use regex to extract 'controller'-postfix including extension
             preg_match(self::FILENAME_REGEX, $name, $matches);
@@ -77,6 +82,10 @@ class ControllersMapGenerator
             $name = str_replace(['_', '/', '\\'], ['-', '--', '--'], $name);
 
             $asset = $this->assetMapper->getAssetFromSourcePath($file->getRealPath());
+            if (!$asset) {
+                throw new \RuntimeException(\sprintf('Could not find an asset mapper path that points to the "%s" controller.', $name));
+            }
+
             $content = file_get_contents($asset->sourcePath);
             $isLazy = preg_match('/\/\*\s*stimulusFetch:\s*\'lazy\'\s*\*\//i', $content);
 


Test patch:
diff --git a/src/StimulusBundle/tests/AssetMapper/ControllersMapGeneratorTest.php b/src/StimulusBundle/tests/AssetMapper/ControllersMapGeneratorTest.php
index 21277b28..15004969 100644
--- a/src/StimulusBundle/tests/AssetMapper/ControllersMapGeneratorTest.php
+++ b/src/StimulusBundle/tests/AssetMapper/ControllersMapGeneratorTest.php
@@ -34,6 +34,8 @@ class ControllersMapGeneratorTest extends TestCase
                     $logicalPath = 'fake-vendor/ux-package1/package-controller-second.js';
                 } elseif (str_ends_with($path, 'package-hello-controller.js')) {
                     $logicalPath = 'fake-vendor/ux-package2/package-hello-controller.js';
+                } elseif (str_ends_with($path, 'other-controller.ts') || str_ends_with($path, 'excluded-controller.js')) {
+                    return null;
                 } else {
                     // replace windows slashes
                     $path = str_replace('\\', '/', $path);
@@ -75,15 +77,18 @@ class ControllersMapGeneratorTest extends TestCase
             $autoImportLocator,
         );
 
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Could not find an asset mapper path that points to the "excluded" controller.');
         $map = $generator->getControllersMap();
         // + 3 controller.json UX controllers
         // - 1 controllers.json UX controller is disabled
-        // + 10 custom controllers (1 file is not a controller & 1 is overridden)
-        $this->assertCount(12, $map);
+        // + 11 custom controllers (1 file is not a controller, 1 is overridden)
+        $this->assertCount(13, $map);
         $packageNames = array_keys($map);
         sort($packageNames);
         $this->assertSame([
             'bye',
+            'excluded',
             'fake-vendor--ux-package1--controller-second',
             'fake-vendor--ux-package2--hello-controller',
             'hello',
diff --git a/src/StimulusBundle/tests/AssetMapper/StimulusControllerLoaderFunctionalTest.php b/src/StimulusBundle/tests/AssetMapper/StimulusControllerLoaderFunctionalTest.php
index 58bb6c8c..346fca2f 100644
--- a/src/StimulusBundle/tests/AssetMapper/StimulusControllerLoaderFunctionalTest.php
+++ b/src/StimulusBundle/tests/AssetMapper/StimulusControllerLoaderFunctionalTest.php
@@ -63,7 +63,8 @@ class StimulusControllerLoaderFunctionalTest extends WebTestCase
                 // 2x from UX packages, which are enabled in controllers.json
                 '/assets/fake-vendor/ux-package1/package-controller-second.js',
                 '/assets/fake-vendor/ux-package2/package-hello-controller.js',
-                // 3x from more-controllers
+                // 4x from more-controllers
+                '/assets/more-controllers/excluded-controller.js',
                 '/assets/more-controllers/hello-controller.js',
                 '/assets/more-controllers/minified-controller.js',
                 '/assets/more-controllers/other-controller.js',
diff --git a/src/StimulusBundle/tests/fixtures/assets/more-controllers/excluded-controller.js b/src/StimulusBundle/tests/fixtures/assets/more-controllers/excluded-controller.js
new file mode 100644
index 00000000..967c4832
--- /dev/null
+++ b/src/StimulusBundle/tests/fixtures/assets/more-controllers/excluded-controller.js
@@ -0,0 +1,6 @@
+// excluded-controller.js
+import { Controller } from '@hotwired/stimulus';
+
+/*    stimulusFetch:    'lazy'   */
+export default class extends Controller {
+}
diff --git a/src/StimulusBundle/tests/fixtures/assets/more-controllers/other-controller.ts b/src/StimulusBundle/tests/fixtures/assets/more-controllers/other-controller.ts
new file mode 100644
index 00000000..1e6f9ccd
--- /dev/null
+++ b/src/StimulusBundle/tests/fixtures/assets/more-controllers/other-controller.ts
@@ -0,0 +1,7 @@
+// other-controller.js
+// @ts-ignore
+import { Controller } from '@hotwired/stimulus';
+
+/*    stimulusFetch:    'lazy'   */
+export default class extends Controller {
+}