Skip to content

Conversation

@RiccardoMargiotta
Copy link
Contributor

@RiccardoMargiotta RiccardoMargiotta commented Nov 28, 2022

Summary

In some cases, it is desirable to have multiple require.context entries and attempt to resolve in each before falling back to global. This PR adds useContexts (in the pattern of useContext) which accepts an array of require.context and attempts to resolve the component/module in each context before falling back to global.

In a larger application, you might find it helpful to split your JavaScript by routes/controllers to avoid serving unused components and improve your site performance by keeping bundles smaller. For example, you might have separate bundles for homepage, search, and checkout routes. In that scenario, you can add multiple require.context component directory paths via useContexts to server_rendering.js, so that you can server side render across all of your routes.

// server_rendering.js
var homepageRequireContext = require.context('homepage', true);
var searchRequireContext = require.context('search', true);
var checkoutRequireContext = require.context('checkout', true);

var ReactRailsUJS = require('react_ujs');
ReactRailsUJS.useContexts([
  homepageRequireContext,
  searchRequireContext,
  checkoutRequireContext
]);

Other Information

This code change was originally authored by @cymen in #1144. I have added additional documentation and brought it up to date with master to help get it released.

@RiccardoMargiotta
Copy link
Contributor Author

I have a failing test on master which persists with this change:

Error:
BundleRendererTest#test_#render_should_pass_prerender_options_to_#before_render:
ArgumentError: mocked method :call expects 3 arguments, got ["Todo", "{\"todo\":\"write tests\"}"]
    minitest (5.16.3) lib/minitest/mock.rb:185:in `method_missing'
    minitest (5.16.3) lib/minitest/mock.rb:302:in `block in stub'
    /Users/e0049252/Documents/repos/react-rails/lib/react/server_rendering/exec_js_renderer.rb:17:in `render'
    /Users/e0049252/Documents/repos/react-rails/lib/react/server_rendering/bundle_renderer.rb:40:in `render'
    /Users/e0049252/Documents/repos/react-rails/test/react/server_rendering/bundle_renderer_test.rb:26:in `block (3 levels) in <class:BundleRendererTest>'
    minitest (5.16.3) lib/minitest/mock.rb:317:in `stub'
    /Users/e0049252/Documents/repos/react-rails/test/react/server_rendering/bundle_renderer_test.rb:25:in `block (2 levels) in <class:BundleRendererTest>'
    minitest (5.16.3) lib/minitest/test.rb:98:in `block (3 levels) in run'
    minitest (5.16.3) lib/minitest/test.rb:195:in `capture_exceptions'
    minitest (5.16.3) lib/minitest/test.rb:95:in `block (2 levels) in run'
    minitest (5.16.3) lib/minitest.rb:296:in `time_it'
    minitest (5.16.3) lib/minitest/test.rb:94:in `block in run'
    minitest (5.16.3) lib/minitest.rb:391:in `on_signal'
    minitest (5.16.3) lib/minitest/test.rb:243:in `with_info_handler'
    minitest (5.16.3) lib/minitest/test.rb:93:in `run'
    minitest (5.16.3) lib/minitest.rb:1059:in `run_one_method'
    minitest (5.16.3) lib/minitest.rb:365:in `run_one_method'
    minitest (5.16.3) lib/minitest.rb:352:in `block (2 levels) in run'
    minitest (5.16.3) lib/minitest.rb:351:in `each'
    minitest (5.16.3) lib/minitest.rb:351:in `block in run'
    minitest (5.16.3) lib/minitest.rb:391:in `on_signal'
    minitest (5.16.3) lib/minitest.rb:378:in `with_info_handler'
    minitest (5.16.3) lib/minitest.rb:350:in `run'
    railties (6.0.2.1) lib/rails/test_unit/line_filtering.rb:10:in `run'
    minitest (5.16.3) lib/minitest.rb:182:in `block in __run'
    minitest (5.16.3) lib/minitest.rb:182:in `map'
    minitest (5.16.3) lib/minitest.rb:182:in `__run'
    minitest (5.16.3) lib/minitest.rb:159:in `run'
    minitest (5.16.3) lib/minitest.rb:83:in `block in autorun'

@RiccardoMargiotta
Copy link
Contributor Author

RiccardoMargiotta commented Nov 28, 2022

Might need a bit of a rethink, at least as far as my usage goes. I think the problem stems back to comments you can see in this issue (#885), where it's possible to have multiple ReactUJS instances mounted.

So imagine a scenario where we have three entry points to split our JavaScript down, plus the server_rendering.js for server-side rendering (SSR):

  • application.js - a general collection of shared components used across a site
  • search.js - components used specifically on the search
  • checkout.js - components used specifically on the search
  • server_rendering.js - an updated file making use of useContexts to load multiple routes, allowing for server rendering on all sections of the site

Say we're in checkout, and loading the checkout.js bundle only. (We're on a page with deliberately reduced functionality, so we don't need the shared components.) This works fine, we get SSR and then we load only the checkout pack to hydrate our components.

But then when we're on the search page, we have an issue. We get SSR on the page, but then we try to load both the shared application pack and the route-specific search pack. At this point, we wind up with two instances of ReactUJS (at least, I think this is the issue), and although we have the initial markup on screen, it's never hydrated with the client-side JS and we wind up with errors like Cannot find component.


[Edit] Maybe I'm just thinking about this the wrong way. Perhaps it's better to have just a single pack tag per route, making the shared components an included directory within them, rather than a separate entry point / pack. So going back to my scenario above, I could have the following entry points which load multiple component directories making use of the new useContexts functionality:

  • application.js - an all-rounder pack, loads shared
  • search.js - loads shared, search
  • checkout.js - loads checkout
  • server_rendering.js - loads shared, search, checkout

Then I can continue to make use of split chunks to extract common modules between route-split packs.

Moving to a single pack tag method would also be closer to the way Shakapacker behaves.


[Edit 2] Yep, that works, and is probably better anyway. 😃 I suspect I was trying to "micro-manage" Webpack too much here by defining my own shared bundle, when that's exactly what split chunks config is for. I might need to tweak the wording of my README changes a little to better communicate the use case. But for route-based entry points, particularly on larger apps, this is looking like a great help.

@cymen
Copy link
Contributor

cymen commented Nov 28, 2022

@RiccardoMargiotta Thanks for taking this on -- I'm just catching up.

@RiccardoMargiotta
Copy link
Contributor Author

@RiccardoMargiotta Thanks for taking this on -- I'm just catching up.

Hey @cymen! If you'd prefer, I'd be more than happy for you to bring your PR up to date with master, and use my suggested documentation changes if you like. I absolutely don't want to take credit for your changes, but I'd love to see them released - this would be hugely helpful for my application!

@RiccardoMargiotta
Copy link
Contributor Author

Closed as we'll continue this work in the original PR #1144. 🎉

@RiccardoMargiotta RiccardoMargiotta deleted the use-contexts branch November 28, 2022 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants