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

Finish Upgrading Webpacker to v5 #637

Merged
merged 3 commits into from Sep 23, 2020
Merged

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Sep 21, 2020

Context

Summary of Changes

  • Gemfile[.lock]: Bump the webpacker gem to version 5.2.1 (~> 5 semver range).
    • Update various config files to the new default configs as of webpacker 5.2.1
  • package.json, yarn.lock: Bump webpack-dev-server from 3.9.0 to 3.11.0 (^3.11.0 semver range).

Checklist

  • Tested Mobile Responsiveness
  • Added Unit Tests
  • CI Passes
  • Deploys to Heroku on test Correctly (Maintainers will handle)
  • Added Documentation (Service and Code when required)

Ran "rails webpacker:install" and committed the changes.

In "config/webpacker.yml,"
kept the ".js.erb" entry under "extensions:"
so that "app/javascript/packs/lib/maps.js.erb" is included.

In "config/webpack/environment.js,"
kept jQuery and rails-erb-loader configs,
as we are still using these.
Commit the changes from running `rails webpacker:install`,
but using a caret "^" semver range for "@rails/webpacker",
rather than an exact version.

(So that we get updates in the 5.x series)
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Sep 21, 2020

Planning to merge this today, and then re-work #636 on top of this.

This Pull Request incidentally makes sure we can install JS dependencies with custom resolutions (a Yarn feature). Which will be helpful for #636.

(Without that, and taking advantage of the custom Yarn dependency resolutions, we would be unable to run Refuge locally/in Docker under the development Rails environment setting. There would be a complaint about "Your Yarn packages are out of date! Please run yarn install to update."

The "integrity check" is something the Yarn developers advocate not doing, because it's old, deprecated code, and the checks it does aren't really necessary. It's dropped from the next major version of Yarn, Yarn 2.0. If you have a lockfile, as we do at this repository, the proper way to do this would be yarn install --frozen-lockfile. For more details about why webpacker dropped the dubious "integrity check", see: rails/webpacker#2518)

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Sep 23, 2020

Also commenting this for the record:

There is a proper process to upgrade Webpacker (as others have done before at this repo). For the sake of documenting this, the proper process is:

  • Upgrade the webpacker Ruby gem in Gemfile and Gemfile.lock.
  • Run rails webpacker:install
    • Review any conflicts in the files updated during this process.
      • If we do something custom at our repository, determine why we deviated from the defaults, and if we need to keep the custom behavior or can drop it in favor of the defaults.
      • Reviewing the webpacker changelog can be helpful for determining which changes were in the webpacker gem itself, versus changes developers at this repository deliberately made. https://github.com/rails/webpacker/blob/5-x-stable/CHANGELOG.md
    • The rails webpacker:install process also upgrades @rails/webpacker in package.json and yarn.lock. Review these updated JavaScript dependencies and see if it is done right, or if you would have done it differently.
      • For example: when I was working on this pull request, the rails webpacker:install process upgraded @rails/webpacker to an exact version, whereas I prefer to use caret ^ server ranges (which means "allow updated versions greater than or equal to the specified version, but only allow updates from the same major version.").

@DeeDeeG DeeDeeG merged commit 0285845 into develop Sep 23, 2020
@DeeDeeG DeeDeeG deleted the finish-upgrading-webpacker branch September 23, 2020 17:37
@DeeDeeG DeeDeeG added packages dependencies Pull requests that update a dependency file labels Oct 8, 2020
@DeeDeeG DeeDeeG mentioned this pull request Oct 31, 2020
5 tasks
DeeDeeG added a commit that referenced this pull request Nov 1, 2020
* Finish Upgrading Webpacker to v5 (#637)
  - Gemfile[.lock]: Update webpacker to 5.2.1
  - Webpacker: Update config files
      Ran "rails webpacker:install" and committed the changes.

      In "config/webpacker.yml,"
      kept the ".js.erb" entry under "extensions:"
      so that "app/javascript/packs/lib/maps.js.erb" is included.

      In "config/webpack/environment.js,"
      kept jQuery and rails-erb-loader configs,
      as we are still using these.
  - JS dependencies: Upgrade webpack-dev-server
      Commit the changes from running `rails webpacker:install`,
      but using a caret "^" semver range for "@rails/webpacker",
      rather than an exact version.

      (So that we get updates in the 5.x series)


* Update dependencies for September 2020 (#636)
  - dependencies: Remove `swagger` node module
      We don't actually use this.

      Also removes 200 sub-dependencies,
      and makes our yarn.lock file about 1270 lines shorter!
  - yarn.lock: Upgrade bootstrap to 4.5.2
      (Was at version 4.4.1)
  - dependencies: Resolve node-fetch to ^2.6.1
  - yarn.lock: Upgrade "selfsigned" and "node-forge"
  - Gemfile[.lock]: Update Rails and dependencies
      rails was 5.2.4.3, now it's version 5.2.4.4


* Keeping filters state on pagination (#638)
  - Keeping filters state on pagination
  - Fix Code Climates complaints.
  - Fixed active issue on ada filter buttons
  - Fixed undefined function problem when run rspec.


* Add rubocop and resolve lint errors (#644)
  - Initialize rubocop
  - Run rubocop with --fix
  - Don't require magic comment
      This will affect every file and have potential side effects, so I'm
      going to start with this turned off.
  - Resolve lint errors in `app`
  - RSpec linting
  - Resolve remaining
  - Fix wrong change
  - Singleton method for verify
  - New lint rules
  - Add rubocop to travis config
  - Correct docker compose command
  - Check for geo
      The condition was actually supposed to be `if geo = results.first`,
      because that's not always obvious the intention and fails lint, I'm just
      doing a truthy check for it which should be the same.
  - Fix typo
  - Add contributing docs for rubocop


* Applying ADA filters for the Map view of search. (#651)
  - Applying ADA filters for the Map view of search.
  - Changed comment for polyfill URLSearchParams
  - Fixed bug of map marker content not showing.


* Add a standard EditorConfig configuration for every contributor to follow (#649)


* Enhancement/i18n thread safe (#647)
  - Provide a way to respond requests without thread-safe issues
      As rails docs says https://guides.rubyonrails.org/i18n.html#managing-the-locale-across-requests
      When we use 'I18n.locale =' the current locale could leak into following requests
      this is a standard and recommended way to deal with it :)
  - Create a shared_example to test I18n locale switching
  - Update spec/controllers/pages_controller_spec.rb


* Upgrade puma to latest version 5.x (#641)


* yarn.lock: Update "http-proxy" to v1.18.1 (fe195d7)


Co-authored-by: Lucas <torres.giorgio@gmail.com>
Co-authored-by: Tegan Rauh <3896310+tegandbiscuits@users.noreply.github.com>
Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant