stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Webpack compilation fails with rails/webpacker 3.6 #83

Closed julianrubisch closed 4 years ago

julianrubisch commented 5 years ago

Bug Report

Before you go on reading: I'm not even sure stimulus_reflex is intended to be backwards compatible. If that isn't so, feel free to close.

With the temporary fix mentioned in https://github.com/hopsoft/stimulus_reflex/issues/81 and https://github.com/hopsoft/stimulus_reflex/pull/82 I was able to install stimulus_reflex, but the pack couldn't be compiled.

Describe the bug

Apparently babel-plugin-transform-object-rest-spread doesn't like the line

controller.StimulusReflex = { ...options, channel, room }

Output of bin/rails webpacker:compile

ERROR in ./node_modules/stimulus_reflex/stimulus_reflex.js
Module parse failed: Unexpected token (168:32)
You may need an appropriate loader to handle this file type.
|   const channel = 'StimulusReflex::Channel'
|   const room = options.room || controller.element.dataset.room || ''
|   controller.StimulusReflex = { ...options, channel, room }
|   extendStimulusController(controller)
|   createSubscription(controller)
 @ ./app/javascript/packs/application.js 14:0-45

To Reproduce

Set up a rails 5.2 application as outlined here: https://github.com/hopsoft/stimulus_reflex/issues/81 and try to bin/rails webpacker:compile

Expected behavior

The pack should compile.

Versions

Ruby: ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-darwin18]
Rails: 5.2.3
Webpacker: 3.6.0
Node: v10.15.3
Yarn: 1.19.1

@rails/webpacker:
stimulus_reflex_test@ {...}/stimulus_reflex_test
└── @rails/webpacker@3.5.5

Is bin/webpack present?: true
Is bin/webpack-dev-server present?: true
Is bin/yarn present?: true
hopsoft commented 5 years ago

I'm not even sure stimulus_reflex is intended to be backwards compatible.

This is great feedback. I'd like to support Rails apps 5.2+ with webpacker.

julianrubisch commented 5 years ago

I'd be glad to help out, but I'm no webpack wizard :-/

andrewmcodes commented 4 years ago

@julianrubisch any chance you have a reproduction example app?

julianrubisch commented 4 years ago

affirmative: https://github.com/julianrubisch/stimulus_reflex_rails5_2_webpacker_3_test

nicpalard commented 4 years ago

We had the same issue. Following the guide to upgrade webpack to version 4.x https://github.com/rails/webpacker/blob/master/docs/v4-upgrade.md solved the problem for us.

julianrubisch commented 4 years ago

@nicpalard I'm sure of that! However, with the app I'm currently working at, that's just not an option :(

andrewmcodes commented 4 years ago

@hopsoft I need a hand with testing this but we may be able to just include the correct babel plugin and make the code backwards compatible.

  "plugins": [
        '@babel/plugin-proposal-object-rest-spread', { useBuiltIns: true },
    ],

It may be worth noting that stimulus' docs have instructions that are not compatible by default with Webpacker 3.5 (see here).

@julianrubisch I am curious why migrating to webpacker 4 is not an option? It is a relatively painless upgrade. I am sure you have your reasons, I am just curious as to what they are exactly from an educational standpoint 🙂

The Babel 6 -> 7 jump is what ruined all my attempted fixes in the rails app without changing the library. I forgot what a huge change that was.

julianrubisch commented 4 years ago

@andrewmcodes oh that‘s easy: it breaks other dependencies that aren’t webpacker 4 ready (in my specific case a problem with web workers)

leastbad commented 4 years ago

While I do think that ES6 + webpack + yarn/npm are an improvement on the jQuery era in general (ie. "when it all works") its stories like @julianrubisch's that make me angsty about what Sam Stephenson aptly referred to as "an unfortunate step backwards for developer ergonomics" generally associated with bikeshedding in the SPA era.

julianrubisch commented 4 years ago

Couldn’t agree more. Transpile all the things!

leastbad commented 4 years ago

Where does this issue stand, given that v3 is now Rails 6 only?

leastbad commented 4 years ago

Without prescribing anything, how do we feel about closing this issue for now?

leastbad commented 4 years ago

Closing this for now. Just housekeeping. Please re-open if it's on your radar.