ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

be cool with too many access tokens in settings 

fixes USERS-369

the large amount of Access Tokens was making ERB rendering take longer
than it did for the JS to be executed, while that was expecting elements
to be in the DOM that weren't.

Instead of wrapping the whole file itself (profile.js) to execute on
DOMContentLoaded or such, I opted to introduce a new API parallel to
js_bundle that abstracts this if only to preserve the history of that
file.

| TEST PLAN |
| ---- ---- |

- before you check out this patch, repro the issue by creating a ton of
  access tokens in the console and visit the settings page

    1000.times { User.first.access_tokens.create! }

- don't open your dev console, go to the settings page and hit Edit
  Settings and verify it does nothing

- check out the patch, rerun webpack and (hard) reload the page
- Edit Settings should be ok now

Change-Id: I51bb8cdb9cd91107166c6d4c0835c3709a6f10f0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/239958
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
Product-Review: Charley Kline <ckline@instructure.com>
QA-Review: Charley Kline <ckline@instructure.com>
This commit is contained in:
Ahmad Amireh 2020-06-11 10:07:43 -06:00
parent 40d4a91e23
commit 8fcd947b72
4 changed files with 29 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
app/controllers/application_controller.rb
View File

@ -2253,12 +2253,30 @@ class ApplicationController < ActionController::Base
def js_bundle(*args)
opts = (args.last.is_a?(Hash) ? args.pop : {})
Array(args).flatten.each do |bundle|
js_bundles << [bundle, opts[:plugin]] unless js_bundles.include? [bundle, opts[:plugin]]
js_bundles << [bundle, opts[:plugin], false] unless js_bundles.include? [bundle, opts[:plugin], false]
end
nil
end
helper_method :js_bundle
# Like #js_bundle but delay the execution (not necessarily the loading) of the
# JS until the DOM is ready. Equivalent to doing:
#
# $(document).ready(() => { import('path/to/bundles/profile.js') })
#
# This is useful when you suspect that the rendering of ERB/HTML can take a
# long enough time for the JS to execute before it's done. For example, when
# a page would contain a ton of DOM elements to represent DB records without
# pagination as seen in USERS-369.
def deferred_js_bundle(*args)
opts = (args.last.is_a?(Hash) ? args.pop : {})
Array(args).flatten.each do |bundle|
js_bundles << [bundle, opts[:plugin], true] unless js_bundles.include? [bundle, opts[:plugin], true]
end
nil
end
helper_method :deferred_js_bundle
def add_body_class(*args)
@body_classes ||= []
raise "call add_body_class for #{args} in the controller when using streaming templates" if @streaming_template && (args - @body_classes).any?

10
app/helpers/application_helper.rb
View File

@ -236,7 +236,7 @@ module ApplicationHelper
@rendered_js_bundles += new_js_bundles
@rendered_preload_chunks ||= []
preload_chunks = new_js_bundles.map do |(bundle, plugin)|
preload_chunks = new_js_bundles.map do |(bundle, plugin, *)|
key = "#{plugin ? "#{plugin}-" : ''}#{bundle}"
Canvas::Cdn::RevManifest.all_webpack_chunks_for(key)
end.flatten.uniq - @script_chunks - @rendered_preload_chunks # subtract out the ones we already preloaded in the <head>
@ -250,8 +250,12 @@ module ApplicationHelper
# to load that "js_bundle". And by the time that runs, the browser will have already
# started downloading those script urls because of those preload tags above,
# so it will not cause a new request to be made.
concat javascript_tag new_js_bundles.map { |(bundle, plugin)|
"(window.bundles || (window.bundles = [])).push('#{plugin ? "#{plugin}-" : ''}#{bundle}');"
#
# preloading works similarily for window.deferredBundles only that their
# execution is delayed until the DOM is ready.
concat javascript_tag new_js_bundles.map { |(bundle, plugin, defer)|
container = defer ? 'window.deferredBundles' : 'window.bundles'
"(#{container} || (#{container} = [])).push('#{plugin ? "#{plugin}-" : ''}#{bundle}');"
}.join("\n") if new_js_bundles.present?
end
end

2
app/jsx/main.js
View File

@ -94,6 +94,8 @@ if (
})
ready(() => {
(window.deferredBundles || []).forEach(loadBundle)
// This is in a setTimeout to have it run on the next time through the event loop
// so that the code that actually renders the user_content runs first,
// because it has to be rendered before we can check if isMathMLOnPage

2
app/views/profile/profile.html.erb
View File

@ -54,7 +54,7 @@
PROFILE: @user_data,
INTERNATIONAL_SMS_ENABLED: Account.site_admin.feature_enabled?(:international_sms)
%>
<% js_bundle :profile %>
<% deferred_js_bundle :profile %>
<% css_bundle :profile_edit, :pairing_code %>
<% if service_enabled?(:avatars) %>