Skip to content
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

Build action_cable.js with Blade #25119

Merged
merged 2 commits into from
May 24, 2016

Conversation

javan
Copy link
Contributor

@javan javan commented May 23, 2016

  • Compiles action_cable.js with Blade, which essentially uses the same Sprockets setup we had in the rake task. No need to duplicate.
  • Uses sprocket-export's #= export directive to replace the somewhat incomplete implementation

/cc @maclover7 @jeremy

@@ -1,3 +1,4 @@
#= export ActionCable
Copy link
Member

Choose a reason for hiding this comment

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

Does this make blade a runtime requirement for anyone running out of a git clone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Unknown Sprockets directives are ignored so without Blade the default behavior applies (setting window.ActionCable).

@javan javan force-pushed the actioncable/blade-build branch 2 times, most recently from 4fd41a0 to 18f234f Compare May 24, 2016 13:21
@javan
Copy link
Contributor Author

javan commented May 24, 2016

I updated Blade to output the build process and handle cleaning the dist path:

$ bundle exec rake assets:compile
Building assets…
[removed] lib/assets/compiled/action_cable.js
[created] lib/assets/compiled/action_cable.js

eventmachine (1.2.0.1)
eventmachine (1.2.0.1-x64-mingw32)
eventmachine (1.2.0.1-x86-mingw32)
execjs (2.6.0)
Copy link
Member

Choose a reason for hiding this comment

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

@javan I see some downgrades, are they intentional from blade? or the dropping of coffee-script versions, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These dependencies were all added recently in 61483b1 when Blade 0.5.1 was introduced. It turns out Thin (a dependency of Blade) doesn't work with event machine 1.2.0 so I backed away from it in javan/blade@62c8cb0. Blade also removed exact dependencies on coffee-script and sprockets to reduce conflicts with apps that have tighter requirements. So if we want specific versions, we should declare them.

Introduced in d6f2000 and was only used by Action Cable. Now handled by Action Cable’s assets:compile task.
@rafaelfranca rafaelfranca merged commit 04a0e89 into rails:master May 24, 2016
@rafaelfranca
Copy link
Member

Backported in 45083d8

rafaelfranca added a commit that referenced this pull request Jul 4, 2016
@javan
Copy link
Contributor Author

javan commented Jul 4, 2016

Thanks, @rafaelfranca! Is it possible to release actioncable-5.0.1 independently (to fix #25649) or are the Rails gems always released together?

@rafaelfranca
Copy link
Member

They are always released together. If you see the rails.gemspec we ask for the same version of the subgems. https://github.com/rails/rails/blob/master/rails.gemspec#L28.

If it is a critical fix we can do a full release early, but this is more something to @matthewd decide since he is the release manager of the next release.

@javan
Copy link
Contributor Author

javan commented Jul 5, 2016

Cool. I don't think it's critical.

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

Successfully merging this pull request may close these issues.

None yet

5 participants