Put @babel/plugin-proposal-class-properties as peer dependency

[tgalopin]

I think this is the best option: we show the requirement on the package without actually depending on it directly. WDYT @chapterjason ?

[chapterjason]

It is only required as a devDependency because it is used in the babel build process.

https://github.com/symfony/stimulus-bridge/blob/8a81bb4590a9198e398950db5604dc294040ef87/.babelrc#L3

https://github.com/symfony/stimulus-bridge/blob/8a81bb4590a9198e398950db5604dc294040ef87/package.json#L12

On the other side it is actually hoisted via @babel#preset-env

$ yarn remove @babel/plugin-proposal-class-properties
$ yarn why @babel/plugin-proposal-class-properties
yarn why v1.22.10
[1/4] 🤔  Why do we have the module "@babel/plugin-proposal-class-properties"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "@babel/plugin-proposal-class-properties@7.12.1"
info Reasons this module exists
   - "@babel#preset-env" depends on it
   - Hoisted from "@babel#preset-env#@babel#plugin-proposal-class-properties"
info Disk size without dependencies: "20KB"
info Disk size with unique dependencies: "112KB"
info Disk size with transitive dependencies: "10.74MB"
info Number of shared dependencies: 25
✨  Done in 0.54s.

I would not add this as a peerDependency (optional runtime dependency) or dependency (runtime dependency) because it is not required in the runtime.

We have two different runtimes in this case:

1. The webpack-plugin, which doesn't require @babel/plugin-proposal-class-properties.
2. The startStimulusApp shortcut, which only requires stimulus and the @symfony/controllers served from the plugin.

A dependency makes only sense if you have any require or es6 import statement that load that package and that is not the case, that also applies to peerDependencys with the difference that the peerDependency should be checked and only loaded if available.

As I have already written I think it is another issue which has nothing to do with this project.

[tgalopin]

I would not add this as a peerDependency (optional runtime dependency) or dependency (runtime dependency) because it is not required in the runtime.

It kinda does though, as using Stimulus without this dependency doesn't work. I'm looking to avoid users not having the module on their project when they use the Stimulus bridge, as this would mean their Stimulus controllers wouldn't work.

@weaverryan any opinion :) ?

[chapterjason]

@tgalopin In the projects I have, I don't have this issue you described, could you create a reproducer?

[weaverryan]

I'm a little confused by this myself. Technically, the @babel/plugin-proposal-class-properties package is included automatically as part of @babel/preset-env since v7.10.0 - but my impression is that it's only actually used if you have the shippedProposals set to true, which we don't in Encore - https://stimulus.hotwire.dev/handbook/installing#using-babel

And yet, Babel is able to parse the targets = syntax.

The tl;dr is:

A) I agree with @chapterjason that it is not the concern of this lib - it shouldn't be a dependency or peerDependency B) If anywhere, in Encore, we should make sure "things just work". I would think this would mean that we should set the shippedProposals to true... but since things appear to be working anyways (I'm not sure how), I'm not sure if we need to do anything there. I wish I knew what was going on with this - it's super odd to have it working but not know why.

[tgalopin]

Fair enough, let's close this then :)

[weaverryan]

For anyone reading: the class properties works thanks to the webpack.config.js file & the recipe - I totally overlooked this - https://github.com/symfony/stimulus-bridge/pull/16#issuecomment-766095306