Skip to content

Update @rails/actioncable dependency to version >=7.0 #705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 4, 2025

Conversation

adrienpoly
Copy link
Member

I upgraded to Rails 8.0 our app and noticed that we now have 2 versions of actioncable in our bundle due to the dependency of turbo-rails to actioncable ^7.0

This PR loosen the dependency to >=7.0

@adrienpoly adrienpoly force-pushed the loosen-action-cable-version branch from 05af63b to 0850912 Compare December 23, 2024 10:42
@adrienpoly
Copy link
Member Author

I think the CI failures are not directly linked to this PR and are addressed by #706

@dhh
Copy link
Member

dhh commented Mar 3, 2025

This includes changes to the app/js file. Take that's not intended?

@adrienpoly adrienpoly force-pushed the loosen-action-cable-version branch 3 times, most recently from cf4df1e to 0dcf22a Compare March 4, 2025 00:25
@adrienpoly adrienpoly force-pushed the loosen-action-cable-version branch from 468588b to fbd4496 Compare March 4, 2025 00:39
@adrienpoly
Copy link
Member Author

adrienpoly commented Mar 4, 2025

@dhh changes to app/js are the result of runing yarn build.

If I don't include them the CI fails

CleanShot 2025-03-04 at 01 23 04@2x

I have rebased main and ran a new yarn build to remove conflicts

@@ -5533,12 +5533,11 @@ class ConnectionMonitor {
isRunning() {
return this.startedAt && !this.stoppedAt;
}
recordPing() {
recordMessage() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is an actual code change?

@@ -5737,6 +5736,7 @@ Connection.prototype.events = {
return;
}
const {identifier: identifier, message: message, reason: reason, reconnect: reconnect, type: type} = JSON.parse(event.data);
this.monitor.recordMessage();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is this?

@dhh
Copy link
Member

dhh commented Mar 4, 2025

Ah, so the code changes are actually pulled from the npm source?

@adrienpoly
Copy link
Member Author

Ah, so the code changes are actually pulled from the npm source?

Yes these changes comes from the build process

@dhh dhh merged commit 8241ae3 into hotwired:main Mar 4, 2025
6 checks passed
@robbl-as
Copy link

robbl-as commented Jun 2, 2025

Is it possible to get a release with this change? The only workaround is to get @hotwired/turbo-rails from github main or a specific commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants