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

Move as much JS as possible from sprockets to vite #1889

Merged
merged 11 commits into from
Oct 20, 2022
Merged
3 changes: 0 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ gem 'bootstrap', '~> 4.6', '>= 4.6.2'

gem 'sprockets-rails', '>= 3.4.2'

gem 'jquery-rails', "~> 4.3"

gem 'font-awesome-rails', '~> 4.7'

gem "lograge", "< 2"
Expand All @@ -101,7 +99,6 @@ gem "attr_json", "~> 1.0"
gem "traject", ">= 3.5" # to include support for HTTP basic auth in Solr url

gem 'simple_form', "~> 5.0"
gem "cocoon"

gem "browse-everything", "~> 1.2"
gem "qa", "~> 5.2"
Expand Down
7 changes: 0 additions & 7 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ GEM
citeproc-ruby (2.0.0)
citeproc (~> 1.0, >= 1.0.9)
csl (~> 2.0)
cocoon (1.2.15)
coderay (1.1.3)
coercible (1.0.0)
descendants_tracker (~> 0.0.1)
Expand Down Expand Up @@ -296,10 +295,6 @@ GEM
actionview (>= 5.0.0)
activesupport (>= 5.0.0)
jmespath (1.6.1)
jquery-rails (4.5.0)
rails-dom-testing (>= 1, < 3)
railties (>= 4.2.0)
thor (>= 0.14, < 2.0)
json (2.6.2)
jwt (2.5.0)
kaminari (1.2.2)
Expand Down Expand Up @@ -721,7 +716,6 @@ DEPENDENCIES
capybara (>= 2.15)
capybara-screenshot
citeproc-ruby (~> 2.0)
cocoon
content_disposition (~> 1.0)
csl-styles (~> 2.0)
dalli (~> 3.2)
Expand All @@ -739,7 +733,6 @@ DEPENDENCIES
html_aware_truncation (~> 1.0)
irb (>= 1.3.1)
jbuilder (~> 2.5)
jquery-rails (~> 4.3)
kaminari (~> 1.2)
kithe (~> 2.6)
listen (~> 3.3)
Expand Down
23 changes: 6 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,31 +104,20 @@ Then:

## Development Notes

### Javascript
### Assets: Javascript, CSS, etc

We are using Vite.js (an ES6-style JS, https://vite-ruby.netlify.app/) for some javascript, but still using sprockets asset pipeline for other javascript. Our layouts will generally load vite-built pack (with `vite_javascript_tag`) as well as a sprockets compiled file (with `javascript_include_file`). Both will be compiled by `rake assets:precompile` in production, and automatically compiled in dev.
We preferentially use Vite.js (an ES6-style JS, https://vite-ruby.netlify.app/) for our javascript. All local JS and most dependencies are handled by vite. Vite source is at `./app/frontend`.

We are still using sprockets to control our CSS (scss), **not** vite.

* All _local javascript_ is controlled using vite, and you should do that for future JS.
* While things should work without it, you may choose to run the vite dev server in development. Just run: `bundle exec vite dev` in it's own terminal window. If you are working on modifying JS, this may give you quicker iterative builds and better error messages.

* Some general purpose dependencies that probably *could* be via vite are at the moment still via sprockets (and gem dependencies to provide the JS, rather than npm packages).
* jQuery
* Bootstrap
* popper.js for Bootstrap
* rails-ujs
* Some JS is still handled by sprockets. browse_everything (admin-pages-only) and blacklight_range_limit only support inclusion via sprockets. Blacklight -- we have just not yet changed over. So our layouts will load vite-built pack (with `vite_javascript_tag`) as well as a sprockets compiled file (with `javascript_include_file`). Both will be compiled by `rake assets:precompile` in production, and automatically compiled in dev. Sprockets JS lives in `./app/assets/javascripts`.

* Some more rails-y dependencies are also still provided by sprockets asset pipeline, in some cases it is tricky to figure out how to transition to vite (or may not be possible)
* cocoon (used for adding/removing multi-values in edit screens; may not be avail as npm package)
* blacklight ([instructions](https://github.com/projectblacklight/blacklight/wiki/Using-Webpacker-to-compile-javascript-assets) for webpacker, which are still helpful)
* blacklight_range_limit (may not be avail as npm package instead of ruby gem/sprockets?)
* browse-everything (not avail as npm package instead of ruby gem/sprockets)
* Note we build separate JS files for some admin-only JS, that is loaded only in the admin.html.erb layout. That includes an admin.js vite front-end, as well as browse-everything via sprockets.

* You can look at `./app/assets/javascripts/application.js` to see what dependencies are still being provided via the sprockets-asset-pipeline-compiled application.js file, usually with src from a rubygem.
* We are still using sprockets to control our CSS (scss), **not** vite. We would like to switch CSS over to vite too. This would let us the supported go-sass for our SCSS, instead of deprecated libsass in sprockets. We will probably do this at the point we can switch Blacklight JS *and* CSS over to new build chain, both using `blacklight_frontend` NPM package.

* uppy (JS for fancy file upload func) is still being loaded in it's own separate script tag from remote CDN (see admin.html.erb layout). This is recommended against by the uppy docs, and we should transition to providing via webpacker and `import` statement, just including the parts of uppy we need, but haven't figured out how to do that yet (including uppy css)

* While things should work without it, you may choose to run the vite dev server in development. Just run: `bundle exec vite dev` in it's own terminal window. If you are working on modifying JS, this may give you quicker iterative builds and better error messages.

### Development docs

Expand Down
44 changes: 24 additions & 20 deletions app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,30 @@
// This is a manifest file that'll be compiled into application.js, which will include all the files
// listed below.
// This is a SPROCCKETS manifest file that'll be compiled into an application.js
//
// Any JavaScript/Coffee file within this directory, lib/assets/javascripts, or any plugin's
// vendor/assets/javascripts directory can be referenced here using a relative path.
// It can include files via sprockets `//= require` directives in JS comments,
// below.
//
// It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the
// compiled file. JavaScript code in this file should be added after the last require_* statement.
//
// Read Sprockets README (https://github.com/rails/sprockets#sprockets-directives) for details
// about supported directives.
//
//= require rails-ujs
// Our sciencehistory app generally tries to avoid using sprockets for JS now, we
// use vite. This *only* includes assets which cannot be conveniently included via
// vite, for instance from dependencies that *only* provide their assets in manner
// for sprockets inclusion.
//
// This will produce a secondary `application.js` included via rails
// `javascript_include_tag`. For bulk of our JS, see vite stuff
// at ./app/frontend

//= require jquery3

//= require popper
// browse_everything https://github.com/samvera/browse-everything
//
// * JS is at the moment only available via sprockets.
// * The standard way to include `require browse_everything` will also
// include`bootstrap. We want to control bootstrap inclusion ourselves,
// possibly via vite. So we include the sub-parts directly, excluding
// bootstrap.
// * See https://github.com/samvera/browse-everything/issues/411
//

// Note do NOT to try require `bootstrap-sprockets`, while it used to work and be
// recommended by Blacklight, stopped being compatible with Blacklight in 7.22.0
// for mysterious reasons.
//= require bootstrap

//= require cocoon
//= require browse_everything
//= require prevent_use_strict

// Required by Blacklight
// not currently using blacklight 'suggest' func which uses twitter typeahead.
Expand All @@ -34,11 +35,14 @@
// require twitter/typeahead
//= require blacklight/blacklight



// For blacklight_range_limit built-in JS, if you don't want it you don't need
// this:
//= require 'blacklight_range_limit'

//= require_tree .





15 changes: 15 additions & 0 deletions app/assets/javascripts/browse_everything.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// browse_everything https://github.com/samvera/browse-everything
//
// * JS is at the moment only available via sprockets.
// * The standard way to include `require browse_everything` will also
// include`bootstrap. We want to control bootstrap inclusion ourselves,
// possibly via vite. So we include the sub-parts directly, excluding
// bootstrap.
// * See https://github.com/samvera/browse-everything/issues/411

// browse_everything is currently only used on staff/admin pages, so at the moment
// we include this JS on the staff back-end.


//= require jquery.treetable
//= require browse_everything/behavior
13 changes: 0 additions & 13 deletions app/assets/javascripts/cable.js

This file was deleted.

Empty file.
14 changes: 14 additions & 0 deletions app/assets/javascripts/prevent_use_strict.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Blacklight JS has "use strict" as it's first line, which puts our entire
// sprockets manifest into browser JS "strict mode" *if and only if* blacklight
// is the FIRST thing required in the sprocketse manifest!
//
// But that's super confusing and other things in our sprockets may not
// work under strict mode, we don't want to do that.
//
// So we avoid it with this weird hack, by first requiring a JS file that
// doesn't do anything, but can provide a first line that is something
// other than "use strict". PHEW!
//
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode

const prevent_use_strict = "prevent_use_strict";
10 changes: 10 additions & 0 deletions app/frontend/entrypoints/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ console.log('Visit the guide for more information: ', 'https://vite-ruby.netlify
// Example: Import a stylesheet in app/frontend/index.css
// import '~/index.css'


// We're still using rails-ujs for now. Rails-ujs 6.x will auto-start itself on import.
import Rails from '@rails/ujs';

import '../javascript/jquery_setup.js'
import '../javascript/bootstrap_setup.js'

// used by kithe, for forms with add/remove fields
import "@nathanvda/cocoon";

import "../javascript/responsive-tabs/responsive-tabs.js"

import '../javascript/init_popovers.js';
Expand Down
6 changes: 6 additions & 0 deletions app/frontend/javascript/bootstrap_setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Bootstrap 4 needs Popper as a pre-req, and needs it installed in window.Popper
import Popper from 'popper.js';
window.Popper = Popper;

// And now bootstrap 4 itself
import "bootstrap";
12 changes: 12 additions & 0 deletions app/frontend/javascript/jquery_setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Assign global window.$ and window.jQuery becuase we have plenty of code that
// needs to access it like this, including Bootstrap 4, other dependencies,
// possibly some local code still.
//
// And also including code included via sprockets which can find it here.
import $ from 'jquery'
window.jQuery = window.$ = $


// Bootstrap 4 also needs Popper, and needs it installed in window.Popper
import Popper from 'popper.js';
window.Popper = Popper;
11 changes: 8 additions & 3 deletions app/views/layouts/admin.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,18 @@

<%# sprockets 'application' js and css %>
<%= stylesheet_link_tag 'application', media: 'all' %>
<%= javascript_include_tag 'application', defer: true %>

<%# vite 'application' js %>
<%# vite's 'application' js before sprockets later %>
<%= vite_client_tag %>
<%= vite_javascript_tag 'application', defer: true %>

<%# vite 'admin' js, will also include css if needed automatically %>
<%= javascript_include_tag 'application', defer: true %>

<%# browse_everything can only be provided via sprockets, but is currently
only used on admin pages, we require a built file just for it here %>
<%= javascript_include_tag 'browse_everything', defer: true %>

<%# admin-specific JS built by vite, will also include css if needed automatically %>
<%= vite_javascript_tag 'admin', defer: true %>

<%# separate and positioned per lazysizes docs
Expand Down
6 changes: 3 additions & 3 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@

<%= stylesheet_link_tag "application", media: "all" %>

<%= javascript_include_tag "application", defer: true %>

<%# vite 'application.js' #>
<%# vite's application.js before sprockets one later %>
<%= vite_client_tag %>
<%= vite_javascript_tag 'application', defer: true %>

<%= javascript_include_tag "application", defer: true %>

<%= csrf_meta_tags %>
<%= content_for(:head) %>
<!--
Expand Down
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@
"name": "scihist_digicoll",
"private": true,
"dependencies": {
"@nathanvda/cocoon": "^1.2.14",
"@rails/ujs": "^ 6.1.7",
"@shopify/draggable": "^1.0.0-beta.8",
"bootstrap": "^ 4.6.2",
"devbridge-autocomplete": "^1.4.10",
"domready": "^1.0.8",
"jquery": "^ 3.6.0",
"js-yaml": ">= 3.13.1",
"lazysizes": "^5.1.0",
"openseadragon": "^2.4.1",
"popper.js": "^ 1.16.1",
"tom-select": "^1.4.3",
"video.js": "^7.18.1",
"videojs-seek-buttons": "^2.2.0"
Expand Down
15 changes: 15 additions & 0 deletions vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,19 @@ export default defineConfig({
fileName: ".br",
}),
],
// video.js is REALLY BIG, telling vite/rollup to chunk it as it's own
// JS file may include performance, or at least reduce warnings about big
// files -- even if it's still statically imported at page load.
build: {
rollupOptions: {
output: {
manualChunks: {
"video.js": ['video.js', 'videojs-seek-buttons']
}
}
},
// Seems necessary to get sourceMaps in dev autoBuild, which are kind of
// important for being able to debug. https://github.com/ElMassimo/vite_ruby/discussions/285
sourcemap: true
}
})
29 changes: 28 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@
resolved "https://registry.yarnpkg.com/@esbuild/linux-loong64/-/linux-loong64-0.15.10.tgz#78a42897c2cf8db9fd5f1811f7590393b77774c7"
integrity sha512-w0Ou3Z83LOYEkwaui2M8VwIp+nLi/NA60lBLMvaJ+vXVMcsARYdEzLNE7RSm4+lSg4zq4d7fAVuzk7PNQ5JFgg==

"@nathanvda/cocoon@^1.2.14":
version "1.2.14"
resolved "https://registry.yarnpkg.com/@nathanvda/cocoon/-/cocoon-1.2.14.tgz#aaea910e4b9c0d28d5bdcb7f3743617db46b09af"
integrity sha512-WcEt2vVp50de2i7rkD4O+96O1iMtMIcTBNGPocrHfcmHDujKOngoLHFF8Ektgoh8PjwFAJMxx8WyGv0BtKTjxQ==
dependencies:
jquery "^3.3.1"

"@nodelib/[email protected]":
version "2.1.5"
resolved "https://registry.yarnpkg.com/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz#7619c2eb21b25483f6d167548b4cfd5a7488c3d5"
Expand All @@ -40,6 +47,11 @@
"@nodelib/fs.scandir" "2.1.5"
fastq "^1.6.0"

"@rails/ujs@^ 6.1.7":
version "6.1.7"
resolved "https://registry.yarnpkg.com/@rails/ujs/-/ujs-6.1.7.tgz#b09dc5b2105dd267e8374c47e4490240451dc7f6"
integrity sha512-0e7WQ4LE/+LEfW2zfAw9ppsB6A8RmxbdAUPAF++UT80epY+7emuQDkKXmaK0a9lp6An50RvzezI0cIQjp1A58w==

"@shopify/draggable@^1.0.0-beta.8":
version "1.0.0-beta.8"
resolved "https://registry.yarnpkg.com/@shopify/draggable/-/draggable-1.0.0-beta.8.tgz#2eb3c2f72298ce3e53fe16f34ab124cc495cd4fc"
Expand Down Expand Up @@ -104,6 +116,11 @@ argparse@^1.0.7:
dependencies:
sprintf-js "~1.0.2"

"bootstrap@^ 4.6.2":
version "4.6.2"
resolved "https://registry.yarnpkg.com/bootstrap/-/bootstrap-4.6.2.tgz#8e0cd61611728a5bf65a3a2b8d6ff6c77d5d7479"
integrity sha512-51Bbp/Uxr9aTuy6ca/8FbFloBUJZLHwnhTcnjIeRn2suQWsWzcuJhGjKDB5eppVte/8oCdOL3VuwxvZDUggwGQ==

braces@^3.0.2:
version "3.0.2"
resolved "https://registry.yarnpkg.com/braces/-/braces-3.0.2.tgz#3454e1a462ee8d599e236df336cd9ea4f8afe107"
Expand Down Expand Up @@ -365,6 +382,11 @@ jquery@>=1.7:
resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.5.1.tgz#d7b4d08e1bfdb86ad2f1a3d039ea17304717abb5"
integrity sha512-XwIBPqcMn57FxfT+Go5pzySnm4KWkT1Tv7gjrpT1srtf8Weynl6R273VJ5GjkRb51IzMp5nbaPjJXMWeju2MKg==

"jquery@^ 3.6.0", jquery@^3.3.1:
version "3.6.1"
resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.6.1.tgz#fab0408f8b45fc19f956205773b62b292c147a16"
integrity sha512-opJeO4nCucVnsjiXOE+/PcCgYw9Gwpvs/a6B1LL/lQhwWwpbVEVYDZ1FokFr8PRc7ghYlrFPuyHuiiDNTQxmcw==

"js-yaml@>= 3.13.1":
version "3.14.0"
resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-3.14.0.tgz#a7a34170f26a21bb162424d8adacb4113a69e482"
Expand Down Expand Up @@ -467,6 +489,11 @@ pkcs7@^1.0.4:
dependencies:
"@babel/runtime" "^7.5.5"

"popper.js@^ 1.16.1":
version "1.16.1"
resolved "https://registry.yarnpkg.com/popper.js/-/popper.js-1.16.1.tgz#2a223cb3dc7b6213d740e40372be40de43e65b1b"
integrity sha512-Wb4p1J4zyFTbM+u6WuO4XstYx4Ky9Cewe4DWrel7B0w6VVICvPwdOpotjzcf6eD8TsckVnIMNONQyPIUFOUbCQ==

postcss@^8.4.16:
version "8.4.18"
resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.18.tgz#6d50046ea7d3d66a85e0e782074e7203bc7fbca2"
Expand All @@ -479,7 +506,7 @@ postcss@^8.4.16:
process@^0.11.10:
version "0.11.10"
resolved "https://registry.yarnpkg.com/process/-/process-0.11.10.tgz#7332300e840161bda3e69a1d1d91a7d4bc16f182"
integrity sha1-czIwDoQBYb2j5podHZGn1LwW8YI=
integrity sha512-cdGef/drWFoydD1JsMzuFf8100nZl+GT+yacc2bEced5f9Rjk4z+WtFUTBu9PhOi9j/jfmBPu0mMEY4wIdAF8A==

queue-microtask@^1.2.2:
version "1.2.3"
Expand Down