search

leastbad / stimulus-hotkeys

MIT License
134 stars 9 forks source link

Start using @hotwired/stimulus instead of stimulus #15

Closed maunovaha closed 2 years ago

maunovaha commented 2 years ago

Sorry for the weird title for the issue. But the problem I am having is that after upgrading our app to rails 7 and latest stimulus installing the stimulus-hotkeys causes some weird behaviour with the importmaps.

Basically, our importmap.rb contains the following entries:

pin "application", preload: true
pin "@hotwired/turbo-rails", to: "turbo.min.js", preload: true
pin "@hotwired/stimulus", to: "stimulus.min.js", preload: true
pin "@hotwired/stimulus-loading", to: "stimulus-loading.js", preload: true
pin "stimulus-timeago", to: "https://ga.jspm.io/npm:stimulus-timeago@4.0.0/dist/stimulus-timeago.es.js"
...

And after running ./bin/importmap pin stimulus-hotkeys

It tries to add following entries:

Pinning "stimulus-hotkeys" to https://ga.jspm.io/npm:stimulus-hotkeys@1.0.5/dist/index.m.js
Pinning "@hotwired/stimulus" to https://ga.jspm.io/npm:@hotwired/stimulus@3.0.1/dist/stimulus.js
Pinning "stimulus" to https://ga.jspm.io/npm:stimulus@3.0.1/dist/stimulus.js

So there is some weird behaviour here, as you can see it tries to add stimulus twice(?)

If you take a look at stimulus-timeago it recently did similar upgrade and installing that library didn't try to add "stimulus" at all. Here is the link: https://github.com/stimulus-components/stimulus-timeago/compare/v3.0.0...v4.0.0

So I think that imports inside the stimulus-hotkeys should also be in a format of:

import { Controller } from '@hotwired/stimulus' rather than import { Controller } from 'stimulus' ?

At the moment, I cannot get the stimulus-hotkeys to work unless I add two different versions of stimulus to my rails app and I would not like to do that because I feel that there might be some unexpected behaviour.

leastbad commented 2 years ago

Hey Mauno,

I agree with you that it's time to move to the new Stimulus package name. There was a significant amount of drama and frustration around the name change last year, but at this point the right thing to do is embrace the future. With that said, I've just published v2.0.0. Please, let me know if you have any issues.

As for the Stimulus package name itself, you might be surprised that you could have removed @hotwired/stimulus and just used stimulus for everything. That is because we worked very hard to make sure that backwards compatibility was maintained and internally, stimulus was changed to reference @hotwired/stimulus in v3.

The whole renaming thing was a huge pain point with zero upside. There were 170,000 references to import {Controller} from 'stimulus' on GitHub, and DHH didn't seem to care because it didn't impact him. The motivation he cited was "marketing".

Finally, I strongyly urge you to switch from import maps to esbuild. Import maps are not ready for serious use. There are serious issues, including not being able to control the load order, errors causing the entire application bundle to stop processing. esbuild is an excellent evolution from webpacker.

maunovaha commented 2 years ago

Thanks for the fast fix and helpful info regarding the subject 👍

After discussing about the state of importmaps with our team we have decided to continue using webpack at the moment; We recently upgraded to Rails 7 and I was just naively assuming that importmaps would be ready to production use, but I guess that is not the case - which is a shame. Oh well, we will keep eye on importmaps and see if those issues are fixed in the future.

leastbad commented 2 years ago

Right on. FWIW, I am still using Webpacker 5.4.3 in my projects. I want to migrate to esbuild, but I haven't locked in the sweet spot, yet. My issue is that I can't seem to get the watcher to pick up any changes in linked dependencies I happen to be actively developing. Honestly, my frustration with the whole situation remains at 4/5.

I sincerely hope that everything is better this time next year.