Add support for multiple `require.context` with addition of `useContexts` #1144

cymen · 2021-09-03T21:43:48Z

Summary

In some cases, it is desirable to be able to have multiple require.context 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.

Other Information

I was not able to get the test suite to run so I have not been able to add tests yet.

RiccardoMargiotta · 2022-01-07T16:51:20Z

Thanks @cymen, this looks really promising for the way I'm trying to split JS bundles by route, but still use server-side rendering. I ran into the same problem as #1034 where I couldn't use multiple files for SSR, even when I disabled sourcemaps.

Here's a small example from my current setup, with a number of smaller split bundles. At the moment, because of the lack of SSR across all entry points, I've only split out components that I'm happy to be client-side rendered (prerender: false).

Before

server_rendering.js

var componentRequireContext = require.context('components', true);
var ReactRailsUJS = require('react_ujs');
ReactRailsUJS.useContext(componentRequireContext);

enquiry.js

var componentRequireContext = require.context('enquiry', true);
var ReactRailsUJS = require('react_ujs');
ReactRailsUJS.useContext(componentRequireContext);

search.js

var componentRequireContext = require.context('search', true);
var ReactRailsUJS = require('react_ujs');
ReactRailsUJS.useContext(componentRequireContext);

After

server_rendering.js

var componentsRequireContext = require.context('components', true);
var enquiryRequireContext = require.context('enquiry', true);
var searchRequireContext = require.context('search', true);

var ReactRailsUJS = require('react_ujs');
ReactRailsUJS.useContexts([
  componentsRequireContext,
  enquiryRequireContext,
  searchRequireContext
]);

No changes to enquiry or search.

I built a version of the package with your code, and I'm happy to say this lead to components from the enquiry and search entry points being bundled into server_rendering.js, allowing me to switch components from those bundles back to SSR (prerender: true). I was also able to run the test suite, and it doesn't seem like your changes have broken any of the existing tests. 👍

Hey @BookOfGreg, hope you're doing OK! This is a pretty useful addition. 😃

justin808 · 2022-09-09T09:41:56Z

README.md

@@ -161,6 +161,8 @@ ReactRailsUJS.useContext(myCustomContext)

 If `require` fails to find your component, [`ReactRailsUJS`](#ujs) falls back to the global namespace, described in [Use with Asset Pipeline](#use-with-asset-pipeline).

+In some cases, having multiple `require.context` may be desired. If so, an array of `require.context` can be passed to `ReactRailsUJS.useContexts`. With an array of contexts, an attempt will be made to `require` the component from each context before falling back to the global namespace as described above.


Can you provide a more concrete example?

Or maybe point to an example in the codebase?

justin808 · 2022-09-09T09:42:22Z

@cymen, thanks for your contribution. Sorry for the delays to get this merged and shipped. Looks useful!

Please:

rebase on master
add unit tests
add a CHANGELOG.md entry
improve the clarity of the docs

Thanks!

alkesh26 · 2022-10-07T11:54:31Z

@cymen, Do you have the availability to work on the changes suggested by @justin808? Please resolve the conflicts.

@cymen, thanks for your contribution. Sorry for the delays to get this merged and shipped. Looks useful!

Please:

rebase on master

add unit tests

add a CHANGELOG.md entry

improve the clarity of the docs

Thanks!

RiccardoMargiotta · 2022-11-02T16:05:52Z

README.md

@@ -161,6 +161,8 @@ ReactRailsUJS.useContext(myCustomContext)

 If `require` fails to find your component, [`ReactRailsUJS`](#ujs) falls back to the global namespace, described in [Use with Asset Pipeline](#use-with-asset-pipeline).

+In some cases, having multiple `require.context` may be desired. If so, an array of `require.context` can be passed to `ReactRailsUJS.useContexts`. With an array of contexts, an attempt will be made to `require` the component from each context before falling back to the global namespace as described above.


@justin808 Had a go at adding a specific use case for multiple contexts, what do you think? @cymen not sure if there's a different example you had in mind.

Suggested change

In some cases, having multiple `require.context` may be desired. If so, an array of `require.context` can be passed to `ReactRailsUJS.useContexts`. With an array of contexts, an attempt will be made to `require` the component from each context before falling back to the global namespace as described above.

In some cases, having multiple `require.context` entries may be desired. 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 an array of `require.context` component directory paths via `useContexts` to `server_rendering.js`, to allow for [Server-Side Rendering](#server-side-rendering) across your application

```js

// 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

]);

Updated the suggestion based on changes from my other PR to hopefully provide a clearer usage example.

This looks good to me (as mentioned elsewhere). Thanks for fleshing it out. I added another example use case with c5cd6c6 which is the one that sent me down this path of wanting multiple contexts (we've been using a forked version of react-rails in production for quite a while and it's been working well).

Really interesting to hear your use case for this.

justin808 · 2022-11-26T04:47:33Z

@RiccardoMargiotta do you have time to push a new PR that includes your suggestions?

RiccardoMargiotta · 2022-11-28T12:32:18Z

@justin808 I've created #1221. Could you run the workflow please?

cymen · 2022-11-28T18:39:13Z

@RiccardoMargiotta Thanks for pushing this forward. I didn't mean to disappear for a while. That documentation update looks good to me.

RiccardoMargiotta · 2022-11-28T18:59:52Z

No worries! As I said on the new PR, I'd be more than happy to continue on this feature on your existing PR here (but I couldn't work out a way to cherry-pick commits across forks, if that's even possible). Either way, I'm really excited that this is getting closer to a release, it'll be hugely helpful.

cymen · 2022-11-28T19:20:09Z

@RiccardoMargiotta Sure, I'm happy to try -- I rebased my branch to master and cherry picked in your updates. I'm going to reread the documentation changes as the use case I wrote this for is slightly different so it might be useful to add another example there.

Add additional use case for multiple require contexts to documentation.

cymen · 2022-11-28T19:33:17Z

Okay, I've updated the documentation for another use case -- it was a bit hard to condense the use case I wrote this originally for into a few sentences but hopefully it makes sense.

@justin808 Can you run the workflow on this PR?

CHANGELOG.md

Co-authored-by: Riccardo Margiotta <riccardo.margiotta@gmail.com>

README.md

Co-authored-by: Riccardo Margiotta <riccardo.margiotta@gmail.com>

RiccardoMargiotta · 2022-11-28T19:46:36Z

Really interesting to hear your use-case for this, and great to know it's been working well in production for you for so long - definitely a good sign!

Thanks for getting it all sorted on your original branch. I'm really looking forward to working with this on my app, glad I could help nudge it forward a little. What do you think, @justin808?

justin808 · 2023-01-16T23:49:08Z

Should this PR have tests?
What are the risks of this PR?
We need an embedded example Rails app that demonstrates such new functionality, like https://github.com/shakacode/react_on_rails/tree/master/spec/dummy

RiccardoMargiotta · 2023-01-20T14:35:04Z

@cymen It possibly got lost in the merge, but we're now missing line 9 of react_ujs/index.js.

var constructorFromRequireContextsWithGlobalFallback = require("./src/getConstructor/fromRequireContextsWithGlobalFallback")

Without this, we'll fail with:

ReferenceError: constructorFromRequireContextsWithGlobalFallback is not defined

RiccardoMargiotta · 2023-01-20T14:40:23Z

@justin808

It should. But it doesn't appear like we have any tests for the react_ujs JS library part of the codebase at present, and quite honestly I wouldn't know where to begin.
I think the risks are relatively low, given that it's an entirely new function based on an existing one. We won't have changed any existing use of the original function, people will have to opt-in to making use of this new feature.
I'll try to create a sample app. Not something I've done before for a project like this, but I'll try to follow existing examples.

cymen · 2023-01-22T03:54:32Z

@RiccardoMargiotta Oops, I think that did go missing in the merge. I've fixed that. I'll take a look at the merge conflict on the changelog.

cymen · 2023-01-22T04:41:04Z

I took a look at adding testings -- I saw the test/dummy_webpacker3 directory and tried running that (after updating the ffi gem in order to get this running on a Mac M1). My thinking being that making a copy of this and using multiple contexts in the copy would be one way to go. However, it looks like the dummy Rails apps are not working.

For test/dummy_webpacker3 when running bundle exec rails server and attempting to access URL http://localhost:3000/pages/hi in a browser:

uninitialized constant PagesController::WebpackerHelpers
Rails.root: /Users/cymen/dev/react-rails/test/dummy_webpacker3

[Application Trace](http://localhost:3000/pages/hi#) | [Framework Trace](http://localhost:3000/pages/hi#) | [Full Trace](http://localhost:3000/pages/hi#)
[app/controllers/pages_controller.rb:2:in `<class:PagesController>'](http://localhost:3000/pages/hi#)
[app/controllers/pages_controller.rb:1:in `<top (required)>'](http://localhost:3000/pages/hi#)

For test/dummy_sprockets when running bundle exec rails server:

~/dev/react-rails/test/dummy_sprockets (master) % bundle exec rails server
Traceback (most recent call last):
	11: from bin/rails:4:in `<main>'
	10: from bin/rails:4:in `require'
	 9: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/railties-6.0.2.1/lib/rails/commands.rb:18:in `<top (required)>'
	 8: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/railties-6.0.2.1/lib/rails/command.rb:46:in `invoke'
	 7: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/railties-6.0.2.1/lib/rails/command/base.rb:69:in `perform'
	 6: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
	 5: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
	 4: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
	 3: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/railties-6.0.2.1/lib/rails/commands/server/server_command.rb:138:in `perform'
	 2: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/railties-6.0.2.1/lib/rails/commands/server/server_command.rb:138:in `tap'
	 1: from /Users/cymen/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/railties-6.0.2.1/lib/rails/commands/server/server_command.rb:141:in `block in perform'
/Users/cymen/dev/react-rails/test/dummy_sprockets/config/application.rb:9:in `<top (required)>': uninitialized constant SprocketsHelpers (NameError)

I agree with @RiccardoMargiotta that is hard to know where to start. I also agree that there is very little risk because we're expanding the API by adding useContexts not changing the existing behavior by modifying the current useContext.

justin808 · 2023-02-14T07:47:30Z

@cymen and @RiccardoMargiotta, should we merge?

I can merge and put this into a 2.7 release.

cymen · 2023-02-14T08:09:53Z

@justin808 Sure, I think it's ready. Thanks!

RiccardoMargiotta · 2023-02-14T08:12:21Z

@justin808 I'd be good with merging! Like Cymen, I've forked out my own version of the JS package with this change, and have been using it in production for a while.

For my use case of route-based packs, it's been working great.

justin808 · 2023-02-14T08:48:26Z

Thanks @RiccardoMargiotta and @cymen!

RiccardoMargiotta · 2023-02-14T09:14:46Z

Thanks @justin808! Appreciate your efforts on this project. 👍

cymen mentioned this pull request Sep 6, 2021

react_component console errors with multiple packs on a page #885

Closed

RiccardoMargiotta mentioned this pull request Jan 7, 2022

Unable to specify multiple server rendering files when using web packer dev server #1034

Closed

icebox approved these changes Jul 5, 2022

View reviewed changes

justin808 reviewed Sep 9, 2022

View reviewed changes

justin808 added this to the 2.7 milestone Oct 19, 2022

justin808 added the enhancement label Oct 19, 2022

RiccardoMargiotta reviewed Nov 2, 2022

View reviewed changes

RiccardoMargiotta mentioned this pull request Nov 28, 2022

Add support for multiple require.context with addition of useContexts (2) #1221

Closed

cymen and others added 6 commits November 28, 2022 14:13

add useContexts which takes an array of require.context

Verified

This commit was signed with the committer’s verified signature.

amomchilov Alexander Momchilov

GPG key ID: 3EB1A96C0BF7BC06

Verified
Learn about vigilant mode

b9c824d

add note to README about useContexts

2c7ca31

Update README to include information about useContexts.

99ded57

Update changelog.

6c5449f

Update usage example.

f12b216

Update usage example to better explain route-splitting and SSR.

402310d

cymen force-pushed the master branch from cce55b2 to 402310d Compare November 28, 2022 19:18

Update README.md

c5cd6c6

Add additional use case for multiple require contexts to documentation.

RiccardoMargiotta reviewed Nov 28, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

c893444

Co-authored-by: Riccardo Margiotta <riccardo.margiotta@gmail.com>

RiccardoMargiotta reviewed Nov 28, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

f92b5f1

Co-authored-by: Riccardo Margiotta <riccardo.margiotta@gmail.com>

add missing import (overlooked in prior merge)

108ed6e

Merge branch 'master' into master

1b739a2

justin808 merged commit b6acd36 into reactjs:master Feb 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multiple `require.context` with addition of `useContexts` #1144

Add support for multiple `require.context` with addition of `useContexts` #1144

cymen commented Sep 3, 2021

RiccardoMargiotta commented Jan 7, 2022

justin808 Sep 9, 2022

justin808 commented Sep 9, 2022

alkesh26 commented Oct 7, 2022

RiccardoMargiotta Nov 2, 2022 •

edited

Loading

RiccardoMargiotta Nov 28, 2022

cymen Nov 28, 2022

RiccardoMargiotta Nov 28, 2022

justin808 commented Nov 26, 2022

RiccardoMargiotta commented Nov 28, 2022

cymen commented Nov 28, 2022

RiccardoMargiotta commented Nov 28, 2022

cymen commented Nov 28, 2022

cymen commented Nov 28, 2022

RiccardoMargiotta commented Nov 28, 2022

justin808 commented Jan 16, 2023

RiccardoMargiotta commented Jan 20, 2023

RiccardoMargiotta commented Jan 20, 2023

cymen commented Jan 22, 2023

cymen commented Jan 22, 2023 •

edited

Loading

justin808 commented Feb 14, 2023

cymen commented Feb 14, 2023

RiccardoMargiotta commented Feb 14, 2023

justin808 commented Feb 14, 2023

RiccardoMargiotta commented Feb 14, 2023

		@@ -161,6 +161,8 @@ ReactRailsUJS.useContext(myCustomContext)

		If `require` fails to find your component, [`ReactRailsUJS`](#ujs) falls back to the global namespace, described in [Use with Asset Pipeline](#use-with-asset-pipeline).

		In some cases, having multiple `require.context` may be desired. If so, an array of `require.context` can be passed to `ReactRailsUJS.useContexts`. With an array of contexts, an attempt will be made to `require` the component from each context before falling back to the global namespace as described above.

-In some cases, having multiple `require.context` may be desired. If so, an array of `require.context` can be passed to `ReactRailsUJS.useContexts`. With an array of contexts, an attempt will be made to `require` the component from each context before falling back to the global namespace as described above.
+In some cases, having multiple `require.context` entries may be desired. 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 an array of `require.context` component directory paths via `useContexts` to `server_rendering.js`, to allow for [Server-Side Rendering](#server-side-rendering) across your application
+```js
+// 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
+]);

Add support for multiple require.context with addition of useContexts #1144

Add support for multiple require.context with addition of useContexts #1144

Conversation

cymen commented Sep 3, 2021

Summary

Other Information

RiccardoMargiotta commented Jan 7, 2022

Before

After

justin808 Sep 9, 2022

Choose a reason for hiding this comment

justin808 commented Sep 9, 2022

alkesh26 commented Oct 7, 2022

RiccardoMargiotta Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

RiccardoMargiotta Nov 28, 2022

Choose a reason for hiding this comment

cymen Nov 28, 2022

Choose a reason for hiding this comment

RiccardoMargiotta Nov 28, 2022

Choose a reason for hiding this comment

justin808 commented Nov 26, 2022

RiccardoMargiotta commented Nov 28, 2022

cymen commented Nov 28, 2022

RiccardoMargiotta commented Nov 28, 2022

cymen commented Nov 28, 2022

cymen commented Nov 28, 2022

RiccardoMargiotta commented Nov 28, 2022

justin808 commented Jan 16, 2023

RiccardoMargiotta commented Jan 20, 2023

RiccardoMargiotta commented Jan 20, 2023

cymen commented Jan 22, 2023

cymen commented Jan 22, 2023 • edited Loading

justin808 commented Feb 14, 2023

cymen commented Feb 14, 2023

RiccardoMargiotta commented Feb 14, 2023

justin808 commented Feb 14, 2023

RiccardoMargiotta commented Feb 14, 2023

Add support for multiple `require.context` with addition of `useContexts` #1144

Add support for multiple `require.context` with addition of `useContexts` #1144

RiccardoMargiotta Nov 2, 2022 •

edited

Loading

cymen commented Jan 22, 2023 •

edited

Loading