Pefixing all Stimulus-related files with stimulus_

[ThomasLandauer]

Q	A
License	MIT
Doc issue/PR	symfony/webpack-encore-bundle#150

[github-actions]

Thanks for the PR 😍

How to test these changes in your application

1. Define the SYMFONY_ENDPOINT environment variable:

   # On *nix and Mac
   export SYMFONY_ENDPOINT=https://api.github.com/repos/symfony/recipes/contents/index.json?ref=flex/pull-1022
   # On Windows
   SET SYMFONY_ENDPOINT=https://api.github.com/repos/symfony/recipes/contents/index.json?ref=flex/pull-1022

2. Install the package(s) related to this recipe:

   composer req 'symfony/flex:^1.16'
   composer req 'symfony/webpack-encore-bundle:^1.9'

3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

   # On *nix and Mac
   unset SYMFONY_ENDPOINT
   # On Windows
   SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

symfony/webpack-encore-bundle

1.0 vs 1.6

diff --git a/symfony/webpack-encore-bundle/1.6/config/packages/test/webpack_encore.yaml b/symfony/webpack-encore-bundle/1.6/config/packages/test/webpack_encore.yaml
new file mode 100644
index 0000000..02a7651
--- /dev/null
+++ b/symfony/webpack-encore-bundle/1.6/config/packages/test/webpack_encore.yaml
@@ -0,0 +1,2 @@
+#webpack_encore:
+#    strict_mode: false
diff --git a/symfony/webpack-encore-bundle/1.0/config/packages/webpack_encore.yaml b/symfony/webpack-encore-bundle/1.6/config/packages/webpack_encore.yaml
index ce02f6b..a1335c5 100644
--- a/symfony/webpack-encore-bundle/1.0/config/packages/webpack_encore.yaml
+++ b/symfony/webpack-encore-bundle/1.6/config/packages/webpack_encore.yaml
@@ -1,14 +1,25 @@
 webpack_encore:
-    # The path where Encore is building the assets.
-    # This should match Encore.setOutputPath() in webpack.config.js.
+    # The path where Encore is building the assets - i.e. Encore.setOutputPath()
     output_path: '%kernel.project_dir%/public/build'
     # If multiple builds are defined (as shown below), you can disable the default build:
     # output_path: false
 
-    # if using Encore.enableIntegrityHashes() specify the crossorigin attribute value (default: false, or use 'anonymous' or 'use-credentials')
+    # If using Encore.enableIntegrityHashes() and need the crossorigin attribute (default: false, or use 'anonymous' or 'use-credentials')
     # crossorigin: 'anonymous'
 
-    # Cache the entrypoints.json (rebuild Symfony's cache when entrypoints.json changes).
-    # To enable caching for the production environment, creating a webpack_encore.yaml in the config/packages/prod directory with this value set to true
-    # Available in version 1.2
-    #cache: false
+    # Preload all rendered script and link tags automatically via the HTTP/2 Link header
+    # preload: true
+
+    # Throw an exception if the entrypoints.json file is missing or an entry is missing from the data
+    # strict_mode: false
+
+    # If you have multiple builds:
+    # builds:
+        # pass "frontend" as the 3rg arg to the Twig functions
+        # {{ encore_entry_script_tags('entry1', null, 'frontend') }}
+
+        # frontend: '%kernel.project_dir%/public/frontend/build'
+
+    # Cache the entrypoints.json (rebuild Symfony's cache when entrypoints.json changes)
+    # Put in config/packages/prod/webpack_encore.yaml
+    # cache: true

1.6 vs 1.9

diff --git a/symfony/webpack-encore-bundle/1.6/assets/app.js b/symfony/webpack-encore-bundle/1.9/assets/app.js
index bb0a6aa..64dfbe2 100644
--- a/symfony/webpack-encore-bundle/1.6/assets/app.js
+++ b/symfony/webpack-encore-bundle/1.9/assets/app.js
@@ -9,4 +9,4 @@
 import './styles/app.css';
 
 // start the Stimulus application
-import './bootstrap';
+import './stimulus_bootstrap';
diff --git a/symfony/webpack-encore-bundle/1.6/assets/bootstrap.js b/symfony/webpack-encore-bundle/1.9/assets/stimulus_bootstrap.js
similarity index 81%
rename from symfony/webpack-encore-bundle/1.6/assets/bootstrap.js
rename to symfony/webpack-encore-bundle/1.9/assets/stimulus_bootstrap.js
index 58308a6..0960e6c 100644
--- a/symfony/webpack-encore-bundle/1.6/assets/bootstrap.js
+++ b/symfony/webpack-encore-bundle/1.9/assets/stimulus_bootstrap.js
@@ -2,7 +2,7 @@ import { startStimulusApp } from '@symfony/stimulus-bridge';
 
 // Registers Stimulus controllers from controllers.json and in the controllers/ directory
 export const app = startStimulusApp(require.context(
-    '@symfony/stimulus-bridge/lazy-controller-loader!./controllers',
+    '@symfony/stimulus-bridge/lazy-controller-loader!./stimulus_controllers',
     true,
     /\.(j|t)sx?$/
 ));
diff --git a/symfony/webpack-encore-bundle/1.6/assets/controllers/hello_controller.js b/symfony/webpack-encore-bundle/1.9/assets/stimulus_controllers/hello_controller.js
similarity index 55%
rename from symfony/webpack-encore-bundle/1.6/assets/controllers/hello_controller.js
rename to symfony/webpack-encore-bundle/1.9/assets/stimulus_controllers/hello_controller.js
index 8c79f65..076bc81 100644
--- a/symfony/webpack-encore-bundle/1.6/assets/controllers/hello_controller.js
+++ b/symfony/webpack-encore-bundle/1.9/assets/stimulus_controllers/hello_controller.js
@@ -3,14 +3,15 @@ import { Controller } from 'stimulus';
 /*
  * This is an example Stimulus controller!
  *
- * Any element with a data-controller="hello" attribute will cause
- * this controller to be executed. The name "hello" comes from the filename:
+ * Any HTML element with a data-controller="hello" attribute will cause this controller to be executed. Example:
+ * <div data-controller="hello"></div>
+ * The name "hello" comes from the filename:
  * hello_controller.js -> "hello"
  *
  * Delete this file or adapt it for your use!
  */
 export default class extends Controller {
     connect() {
-        this.element.textContent = 'Hello Stimulus! Edit me in assets/controllers/hello_controller.js';
+        this.element.textContent = 'Hello Stimulus! Edit me in assets/stimulus_controllers/hello_controller.js';
     }
 }
diff --git a/symfony/webpack-encore-bundle/1.6/assets/controllers.json b/symfony/webpack-encore-bundle/1.9/assets/stimulus_controllers.json
similarity index 100%
rename from symfony/webpack-encore-bundle/1.6/assets/controllers.json
rename to symfony/webpack-encore-bundle/1.9/assets/stimulus_controllers.json
diff --git a/symfony/webpack-encore-bundle/1.6/config/packages/webpack_encore.yaml b/symfony/webpack-encore-bundle/1.9/config/packages/webpack_encore.yaml
index a1335c5..fa2e39d 100644
--- a/symfony/webpack-encore-bundle/1.6/config/packages/webpack_encore.yaml
+++ b/symfony/webpack-encore-bundle/1.9/config/packages/webpack_encore.yaml
@@ -4,6 +4,16 @@ webpack_encore:
     # If multiple builds are defined (as shown below), you can disable the default build:
     # output_path: false
 
+    # Set attributes that will be rendered on all script and link tags
+    script_attributes:
+        defer: true
+        # Uncomment (also under link_attributes) if using Turbo Drive
+        # https://turbo.hotwired.dev/handbook/drive#reloading-when-assets-change
+        # 'data-turbo-track': reload
+    # link_attributes:
+        # Uncomment if using Turbo Drive
+        # 'data-turbo-track': reload
+
     # If using Encore.enableIntegrityHashes() and need the crossorigin attribute (default: false, or use 'anonymous' or 'use-credentials')
     # crossorigin: 'anonymous'
 
diff --git a/symfony/webpack-encore-bundle/1.9/post-install.txt b/symfony/webpack-encore-bundle/1.9/post-install.txt
new file mode 100644
index 0000000..735ed3e
--- /dev/null
+++ b/symfony/webpack-encore-bundle/1.9/post-install.txt
@@ -0,0 +1,5 @@
+  * Install Yarn and run yarn install
+
+  * Uncomment the Twig helpers in templates/base.html.twig
+
+  * Start the development server: yarn encore dev-server
diff --git a/symfony/webpack-encore-bundle/1.6/webpack.config.js b/symfony/webpack-encore-bundle/1.9/webpack.config.js
index 056b04a..b57f3fc 100644
--- a/symfony/webpack-encore-bundle/1.6/webpack.config.js
+++ b/symfony/webpack-encore-bundle/1.9/webpack.config.js
@@ -23,7 +23,7 @@ Encore
     .addEntry('app', './assets/app.js')
 
     // enables the Symfony UX Stimulus bridge (used in assets/bootstrap.js)
-    .enableStimulusBridge('./assets/controllers.json')
+    .enableStimulusBridge('./assets/stimulus_controllers.json')
 
     // When enabled, Webpack "splits" your files into smaller pieces for greater optimization.
     .splitEntryChunks()

[stof]

This assets/controllers.json file is managed by Flex, so you cannot rename it here.

[ThomasLandauer]

OK. So what can I do?

[stof]

Well, either not renaming the controllers.json file, or first building a way to make this filename configurable in Flex to allow new projects to use the new name while existing projects would still use assets/controllers.json until they migrate.

[ThomasLandauer]

Migrate? I was thinking these recipes here are a one-time installation "helper", and this is (more or less) what's called "Flex" ;-) But obviously there's more to it... So where is this Flex-thing ;-) configured?

[stof]

Flex is composer plugin, that installs recipes and manages a few files automatically (config/bundles.php and assets/controllers.json)

[ThomasLandauer]

Thanks! It's coming from this line: https://github.com/symfony/flex/blob/main/src/PackageJsonSynchronizer.php#L111

So in order to make this change, both repos need to be updated simultaneously?

For existing users it shouldn't make a difference, cause the path in .enableStimulusBridge() (in webpack.config.js) overrides, right?

[stof]

Well, for existing users, they need their Flex plugin to keep managing the assets/controllers.json file until the time they rename it. That's why I said it needs a migration strategy for the Flex plugin.

[ThomasLandauer]

After thinking some more about it, I'm now suggesting:
Do not install Stimulus by default at all

Why:
First and foremost: Because it's not needed by Webpack Encore. People following https://symfony.com/doc/current/frontend/encore/installation.html are looking for a solution "Managing CSS and JavaScript" (as the title says). And then they secretly (!) get a fancy new "framework" installed, with zero documentation?! => This is not good ;-)

So I'm suggesting:
I could create a page somewhere at https://symfony.com/doc/current/frontend.html, telling people

• what Stimulus is
• how to install it manually (i.e. copy-pastable code blocks of all those files I'm trying to rename in this PR)

The result would be:

• Better informed users
• Less users having some JavaScript component included in their built files, which they don't use at all

[weaverryan]

I would be ok with renaming to bootstrap_stimulus.js - I can totally understand that being confused with Twitter Bootstrap.

I don't like renaming controllers/. You can look inside to see what it deals with.

I DO want to continue installing this by default. The benefits outweigh negatives, and making a change like this also invalidates documentation, screencasts & people's existing knowledge of how it works. In other words, change has cost, so it needs to be worth it.

[ThomasLandauer]

The more I think about it, the more absurd this seems to me!
What was the point of inventing Flex in the first place? => Letting people pick what they need (instead of installing everything). And now you're forcing a feature onto users, only because it's cool?!

invalidates documentation

No, cause there is none! ;-) This makes matters even worse! Instead of telling people:

Look, there is this cool library which we love so much that we're installing it by default. If you don't need it, you can disable it by doing...

.. the documentation says: NOTHING. Nothing about what Stimulus is, nothing about installation, nothing about uninstallation, nothing about possible problems.

In my case, Stimulus broke the installation of Webpack Encore, so I ended up opening an issue at symfony/stimulus-bridge#49
So effectively a library that I don't really need delayed switching to Webpack Encore for days...

existing knowledge of how it works

Anybody who already knows how it works is already using it. So they're not affected if it's not installed by default anymore for future users.
And even if: Were just talking about 3-4 copy-pastable files! This takes an average user 2 minutes.

Finally: @weaverryan, thanks a lot for https://symfonycasts.com/screencast/stimulus/controllers ! If this didn't exist, I'll probably still be searching today... ;-)
But in the end, what I'm asking for would just mean to add one more sentence:

To install Stimulus, follow the three short steps at http://...

But maybe I should have asked another question first ;-) What are the benefits of installing it by default?

[weaverryan]

.. the documentation says: NOTHING. Nothing about what Stimulus is, nothing about installation, nothing about uninstallation, nothing about possible problems.

Very fair critique. This is a huge problem.

In my case, Stimulus broke the installation of Webpack Encore, so I ended up opening an issue at symfony/stimulus-bridge#49
So effectively a library that I don't really need delayed switching to Webpack Encore for days...

This is unfortunate, but it's a temporary issue. We need to release v3 support - it's ready. But I want to release one more 1.x symfony/ux release that supports Symfony 6 first (waiting on symfony/panther to add support first).

But maybe I should have asked another question first ;-) What are the benefits of installing it by default?

Developer experience ;). Our hope is that most people that are starting new projects WILL use Stimulus, and then it's already setup and ready for them. Nobody knows what percentage really do, so it's a guessing game. Also, there is not a mechanism in Flex to add these files other than through WebpackEncoreBundle's recipe. So installation would mean copying a few files manually. Not a huge deal - but I still think we should keep it.

We should fix (add) documentation and get our Stimulus v3 release done. That would have prevented the problems in the first place. We don't need to blow up the system because of the bumps :).

[ThomasLandauer]

Also, there is not a mechanism in Flex to add these files other than through WebpackEncoreBundle's recipe.

You mean it's not possible to enable installation in the "usual" way, by doing something like composer require stimulus/stimulus? That's why you came up with the idea to piggyback it onto Webpack Encore?

Let's get a clear picture which files we're talking about:

1. app.js:

   import './bootstrap';

2. webpack.config.js:

   .enableStimulusBridge('./assets/controllers.json')

3. bootstrap.js: Entire file (~10 lines)
4. controllers.json: Entire file (4 lines)

controllers/hello_controller.js isn't really needed.

My opinion about them:

1. This is a total no-go for me! It needs to go, or at least be commented out. Right now, you're effectively bloating people's build.js by secretly adding an unneeded component!
2. If (1) is gone, this isn't needed anymore too ;-) Sure you can fix the current installation issue. But we both know that in the future other issues will arise - just as always with any dependency ;-)

So what I'm suggesting:

• Let's set up a nice page about Stimulus at https://symfony.com/doc/current/frontend.html ! This is the usual way to advertise something - instead of silently tricking it onto people ;-)
• Keep the line in webpack.config.js, but comment it out - just as it's done with all the other libraries there, e.g.:

  // uncomment if you use React
  //.enableReactPreset()

• Same with the line in app.js: // uncomment to enable Stimulus:...
• And for the remaining two (bootstrap.js and controllers.json): I'd prefer to not create them. But if you love them so much :-) - there should at least be a note inside them: "You can safely delete this file if you don't use Stimulus."
  But wait, there's no way to add a comment to JSON (controllers.json) :-( - So?

[ThomasLandauer]

To "fix" symfony/stimulus-bridge#47, there should be the expected package.json (see symfony/stimulus-bridge#50 (comment)) shown on that Stimulus page we're discussing here.

[weaverryan]

instead of silently tricking it onto people

Please be careful - we are not trying to trick anyone. Let's please keep the language objective and constructive :).

To "fix" symfony/stimulus-bridge#47, there should be the expected package.json (see symfony/stimulus-bridge#50
(comment)) shown on that Stimulus page we're discussing here.

You're correct about this, of course. But I don't understand what you mean to say. The WebpackEncoreBundle gives you that package.json, so you should always have core-js already. I think I might be missing something in your comment about this - so please let me know.

Overall, I think your points are indeed valid. However, I still disagree in the final conclusion. The goal is to streamline the "main" experience. That includes people being able to install WebpackEncoreBundle and then install UX packages and have them work. Commenting files out or removing them entirely is going to make it more possible that we break this workflow.

So, let's try to find something that we can agree to fix :). I can think of a few, some are obvious:

A) We need to get the Stimulus v3 support done and taagged!
B) If there is some problem related to symfony/stimulus-bridge#47, we should fix that.
C) We can enhance the comments here so you're more aware what's happening -

recipes/symfony/webpack-encore-bundle/1.9/assets/app.js

Lines 11 to 12 in a624d3b

	// start the Stimulus application
	import './bootstrap';

- and possibly rename this file to bootstrap_stimulus.js.

By the way, I need to check, but I think if you comment-out bootstrap.js, then even if you still have enableStimulusBridge() called in webpack.config.js, it has no effect. I think that method has almost no (or possibly actually no effect) no (it was there for an earlier version of stimulus-bridge). Removing that would be a win.

[ThomasLandauer]

Well, you're installing a software on my machine without telling me. How would you call that?

So what I'd suggest (and I could do) is:
Mention the fact that Stimulus is being installed at Installing Encore in Symfony Applications, and link to either a new Stimulus page in Symfony Docs or to Stimulus' own docs.
What do you say?

[nicolas-grekas]

This narrative that suggests there are nasty intentions embedded in the code makes it very unpleasant to review this discussion.
Kudos to @weaverryan for trying to keep things constructive.

@ThomasLandauer I feel like you're try to say that we should consider decoupling webpack-encore-bundle from symfony-ux but please seriously reconsider the way you express your point.

[ThomasLandauer]

I asked three times if it's OK if I add some documentation, but didn't get an answer. So I'm giving up now. Sorry for having bothered you at all!

[ThomasLandauer]

A day later, let me express my point of view:

When I first questioned the current practice, you said:

I DO want to continue installing this by default.

OK. Then, when I used more drastic words to convince you that what you're doing is a problem, you said:

Please be careful

please seriously reconsider the way you express your point.

OK. Finally, combining the fact that you didn't merge symfony/symfony-docs#14499 in more than a year, with my impression that you're seeing me as not "constructive", then I'm sure you can guess how "high" my motivation is to put more effort in here.

So since you're not seeing as a problem what I'm seeing as the main problem, I don't want to continue being the bad boy who questions your decisions.

[nicolas-grekas]

"secretly", "absurd", "only because it's cool?!", "forcing", "only worse", "bloating", "silently", "tricking", "without telling me"

These are the words you used in this discussion and that made us ask you to keep things cool, because nobody can feel comfortable with such an atmosphere. @weaverryan did try to understand you concerns, despite the tone. Please acknowledge that.

combining the fact that you didn't merge symfony/symfony-docs#14499 in more than a year, with my impression that you're seeing me as not "constructive"

I'm not looking at the doc repo very closely, sorry about that. Other ppl could have chimed into this PR instead of me, but that didn't happen. This has nothing to do with you. And yes, the words I listed above don't contribute to the constructive part of the discussion. I might be wrong but interpret them as expression of frustration, because you really want us to understand your point and you fear we might not. On my side, and I suppose on yours and Ryan's too, we need a peaceful atmosphere to best engage into discussions.

So since you're not seeing as a problem what I'm seeing as the main problem, I don't want to continue being the bad boy who questions your decisions.

We (Ryan mostly) are trying to understand the problem you're raising, but we also don't want to sacrifice the core reasons that made us build things the way they are today. We should be all trying to understand how to fix ones' problem without negating the other side's problem. Your help on this path is very much appreciated. There is no good or "bad boy". Our past decisions always remain open for discussion.

[ThomasLandauer]

Well, thanks for answering!

I'm always trying to see it from the users' perspective. What would you say if you'd type install A, and only later, incidentally discover that you received A plus B, which makes your assets larger? I bet, you would be "astonished" (not to say upset) too, wouldn't you?

And I couldn't believe that you didn't instantly agree that doing it this way is not good. That's why I was using some (maybe too) harsh words. But I certainly didn't want to offend or insult you or anybody else - sorry about that!

So answering your question from above:

I feel like you're try to say that we should consider decoupling webpack-encore-bundle from symfony-ux

I don't know what Symfony UX is exactly, but "decoupling" sounds very good! :-) How could this be achieved? Cause Ryan said above:

Also, there is not a mechanism in Flex to add these files other than through WebpackEncoreBundle's recipe.

This means, that doing it the usual way with something like composer require stimulus/stimulus is not possible in this case? So piggybacking it onto Webpack Encore is the one and only chance?

If yes, I see two options:

1. Continue creating those 4 files (see above) automatically, but comment them out. If this is the way to go, all filename should be prefixed with stimulus_, to make more obvious what they're about (this was the original intent of this PR ;-)
2. Just tell people how cool Stimulus is and let them create/modify those 4 files themselves by copy-paste. This is what I think is better for the users, and also easier to maintain in the future (just change the docs whenever anything in Stimulus changes).

In any case, we all agree that documentation is needed. I think (contrary to what I said above) the best place is a dedicated Stimulus page listed at https://symfony.com/doc/current/frontend.html#other-front-end-articles
Cause (I must finally say this) I still can't see no objective reason why Stimulus is being treated "better" than all the other cool things that Webpack Encore has to offer (asset versioning, Sass, Vue, React, Babel, ...) If it's just for the reason that Ryan said above:

Our hope is that most people that are starting new projects WILL use Stimulus

...then I'd say the best way is to (a) add a note from composer/flex after installation, and/or (b) mention it at https://symfony.com/doc/current/frontend/encore/installation.html