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

Bump sprockets-rails from 3.2.2 to 3.4.2 #5462

Merged
merged 1 commit into from Apr 3, 2024

Conversation

javierm
Copy link
Member

@javierm javierm commented Apr 1, 2024

References

Objectives

  • Avoid breaking CKEditor every time we (or Dependabot) run bundle update <gem> and accidentally update sprockets-rails
  • Make sure we don't break CKEditor when upgrading to Rails 7.0

@javierm javierm added the dependencies Pull requests that updates a dependency label Apr 1, 2024
@javierm javierm self-assigned this Apr 1, 2024
@javierm javierm added this to Reviewing in Consul Democracy Apr 1, 2024
@javierm javierm mentioned this pull request Apr 1, 2024
Note that, since version 3.3.0, sprockets-rails uses a processor to get
digested paths for asset files [1]. This breaks the images used in
CKEditor buttons [2], so we're using the new configuration option [3] to
keep the old behavior.

[1] Pull request 476 in https://github.com/rails/sprockets-rails
[2] Issue 919 in https://github.com/galetahub/ckeditor
[3] Pull request 489 in https://github.com/rails/sprockets-rails
@javierm javierm force-pushed the bump_sprocket_rails_to_3.4.2 branch from 3f65a2d to 89c0ab2 Compare April 3, 2024 13:46
@taitus taitus self-assigned this Apr 3, 2024
Consul Democracy automation moved this from Reviewing to Testing Apr 3, 2024
@javierm javierm merged commit 21b7f4c into master Apr 3, 2024
13 checks passed
Consul Democracy automation moved this from Testing to Release 2.2.0 Apr 3, 2024
@javierm javierm deleted the bump_sprocket_rails_to_3.4.2 branch April 3, 2024 14:41
@javierm
Copy link
Member Author

javierm commented Apr 5, 2024

After merging this pull request, we're getting a warning when compressing assets on production:

WARN -- : Removed sourceMappingURL comment for missing asset
'graphiql/rails/push-pull-async-iterable-iterator.esm.mjs.map' from
gems/graphiql-rails-1.8.0/app/assets/javascripts/graphiql/rails/graphiql-1.4.2.js

There are at least two possible ways to remove this warning.

First, we could start using Terser instead of Uglifier to compress JavaScript. Not sure why, but at some point I thought using Terser removed the warning. Apparently, it doesn't.

Second, we could remove graphiql-rails from production, so it's only available in development. This is actually what's recommended in the graphiql-rails README. We enabled graphiql-rails on production in #1659.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that updates a dependency
Projects
Consul Democracy
  
Release 2.2.0
Development

Successfully merging this pull request may close these issues.

None yet

2 participants