symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
849 stars 312 forks source link

The "lazyness" of symfony/ux-react isn't true? Why my React imports are loaded for every request? #1177

Closed gremo closed 1 month ago

gremo commented 1 year ago

Given this controllers.json (notice the lazy):

{
  "controllers": {
    "@symfony/ux-react": {
      "react": {
        "enabled": true,
        "fetch": "lazy"
      }
    }
  },
  "entrypoints": []
}

The controller symfony--ux-react--react itself will in fact load lazly:

image

The main problem is that every import inside alll React components is loaded on every page request, not only on the page that uses the symfony--ux-react--react controller.

Take the following. It's a page that doesn't load the symfony--ux-react--react at all. I've highlighted the network tab, where you can see that:

image

The question is: is this the intended behaviour of symfony/ux-react, or my fault, or maybe a bug?

Kocal commented 1 year ago

How do you call registerReactControllerComponents?

I remember implementing lazy-loading for Vue components (https://github.com/symfony/ux/pull/482/), maybe this is something we can implement for React aswell?

And to lazy-load components, passing 'lazy' as 4th parameter to require.context() is required registerVueControllerComponents(require.context('@/js/vue/components', true, /\.vue$/, 'lazy'))

/ping @weaverryan do you know why this has been removed from the documentation in https://github.com/symfony/ux/commit/cf6a9bdf9c711d792ff89d0c485cb1761c2f1a50#diff-f2f308b02d020be83e29caf4bd208c5b2fd8ac4547fa337504cddeab362335aaL69-L71? For ImportMap/AssetMapper? :/

gremo commented 1 year ago

@Kocal this:

registerReactControllerComponents(require.context('./react/controllers', true, /\.(j|t)sx?$/));

I'm not aware of the lazy option, I'll try (where is documented btw?).

So it something that isn't supported ATM?

Kocal commented 1 year ago

Yes, it's something which is not implemented with the React integration.

For the 'lazy' parameter, see the 3rd example in https://webpack.js.org/api/module-methods/#requirecontext

weaverryan commented 1 year ago

/ping @weaverryan do you know why this has been removed from the documentation in https://github.com/symfony/ux/commit/cf6a9bdf9c711d792ff89d0c485cb1761c2f1a50#diff-f2f308b02d020be83e29caf4bd208c5b2fd8ac4547fa337504cddeab362335aaL69-L71? For ImportMap/AssetMapper? :/

That was probably accidental - and unrelated to AssetMapper. It related to adding these recipes - https://github.com/symfony/recipes/pull/1213 - so the require.context line is added automatically. So I removed it from the docs, but should have kept a mention of the lazy, if you want to add that.

Yes, it's something which is not implemented with the React integration

Iirc, when @tgalopin added it, he mentioned that the React Suspense feature should be used, instead of us adding laziness ourselves.

tgalopin commented 1 year ago

Yes exactly, there are dedicated tools in React which are better suited to handle the lazyness than ours IMHO

Kocal commented 1 year ago

Does it deserve a documentation section?

gremo commented 1 year ago

So, people, thank you for the clarification. The best way to accomplish this is React.lazy and Suspence, is this correct?

I'm going to close this issue today.

petermanders89 commented 8 months ago

I do not really understand how to properly use lazy incombination with ux-react. Or it does not work? I see that is an older issue, but I think it is still relevant.

A quick example without going to much into depth. I compile using symfony/webpack-encore with only one addEntry('app', './assets/app.js').

I got an app which imports a sub component:

import { lazy, Suspense } from 'react';

const LazyApp = lazy(() => import('./app/lazy-app'));

const testApp = () => {
  return (
    <Suspense>
      <LazyApp />
    </Suspense>
  );
};

export default testApp;

The imported file:

export const LazyApp = () => (
  <h1>Test Header</h1>
);

export default LazyApp;

console.log('LazyApp');

When using registerReactControllerComponents(require.context('./js/tmp', true, /\.(j|t)sx?$/)) I do not expect the console to appear, but it does. If I do a direct import using import './js/tmp/test-app';, I do not get that console .log.

petermanders89 commented 8 months ago

So I came to the conclusion that my implementation is faulty.

For people who also come accross the above. The 2nd (require.context('./js/tmp', true, /\.(j|t)sx?$/) value is the deep search property. This means all subdirectories are searched and compiled. While setting it to false only the root directive (./js/tmp) is compiled.

In my situation I had a couple of full page applications in a js/react folder. But all other components (including redux for example) was living in the sub folders. With the deep property on true the lazy loading implementation didn't work, but with false it does.

I think clarifcation would be needed on the documentation here for the 2nd parameter of the require.context: https://symfony.com/bundles/ux-react/current/index.html

carsonbot commented 2 months ago

Thank you for this issue. There has not been a lot of activity here for a while. Has this been resolved?

carsonbot commented 1 month ago

Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3

carsonbot commented 1 month ago

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!