Skip to content

Declare Trix as peer dependency of Action Text's npm package #34958

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
Jan 17, 2019

Conversation

javan
Copy link
Contributor

@javan javan commented Jan 17, 2019

  1. Change trix from a dependency to a peer dependency since Trix isn't used directly by @rails/actiontext
  2. Tidy up the action_text:install task to install both @rails/actiontext and trix using the versions specified in package.json

javan added 2 commits January 17, 2019 10:42
Automate installing the appropriate packages with yarn and appending them to the default application.js pack.
@rails-bot rails-bot bot added the actiontext label Jan 17, 2019
@@ -1,3 +1,13 @@
require "pathname"
Copy link
Member

Choose a reason for hiding this comment

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

Should this installer still exists? Is not the idea to generate the application and already get actiontext on it like the other frameworks like actioncable?

cc @dhh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, TBH. Active Storage needs to be installed first, which requires running rails active_storage:install (I think?).

Any reason not to merge this in the meantime?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can make that work. Not a blocker for this PR, just asking because this may be something we need to change before the beta2.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the idea is still that this requires running a task if you actually want to use it in the app. Since we do migrations and stuff. Just like active storage. But that everything is already there. Nothing to configure or setup or add dependencies for. Just a single task to run.

@javan javan merged commit 69c963c into rails:master Jan 17, 2019
@javan javan deleted the actiontext/trix-as-peer branch January 17, 2019 18: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.

3 participants