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

Generate cable.js file if does not exist when generating channel #24524

Merged
merged 1 commit into from
Apr 13, 2016

Conversation

prathamesh-sonpatki
Copy link
Member

r? @jeremy

- Before this, while generating a channel, we were not creating
  `cable.js` if it does not already exist.
- We have similar code for application mailer here -
  rails@0b3ae02.
- Based on the comment -
  rails#24418 (comment).

App.cable = ActionCable.createConsumer();

}).call(this);
Copy link
Member Author

Choose a reason for hiding this comment

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

We already have a cable.js in application templates in railties. Should we refer to it instead of duplicating here? I have seen similar duplicate file for application_mailer.rb too in mailer_generator template. We can remove duplicate file there too.

Copy link
Member

Choose a reason for hiding this comment

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

No, we can't. Frameworks can't add dependencies in each other.

@maclover7
Copy link
Contributor

Dup of #24245?

@rafaelfranca rafaelfranca merged commit a162600 into rails:master Apr 13, 2016
@prathamesh-sonpatki
Copy link
Member Author

Jon, this PR was about making sure that we have cable.js present while generating a channel using generator. #24245 is about generating JS version of action cable assets if coffee script is not used in the app.

@prathamesh-sonpatki prathamesh-sonpatki deleted the cablejs-for-channels branch April 13, 2016 02:24
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

4 participants