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

Generate new two version of javascript bundle (app-hash,js) #24906

Closed
ghassanmas opened this issue Jun 10, 2020 · 18 comments
Closed

Generate new two version of javascript bundle (app-hash,js) #24906

ghassanmas opened this issue Jun 10, 2020 · 18 comments
Assignees
Labels
topic: webpack/babel Webpack or babel type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.

Comments

@ghassanmas
Copy link

Summary

This feature is relevant for the build stage
So Gatsby creates a JS bundle which will be used:

  • When Gatsby creates the html pages.
  • When served to the client.
    But suppose some of the packages are only relevant when creating the html pages, (e.g. if when using a package to format a word for localization purpose like react-intl)

Basic example

This could be dont implemented as follows, Suggestion
As a plugin:

  resolve: 'gatsby-exclude-client',
      options: {
        modules: ['@formatjs','react-intl'],
      },
    }

Motivation

I think this would decrease the file size sent to the client, which would lead to better performance.
I have encountered that when testing with lighthouse, hence I got unused script warning, this is a related issue #24332. (I have used webpack bundle analyzer also the dev tools coverage tools) which eventually lead me to this conclusion.

@ghassanmas ghassanmas added the type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. label Jun 10, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 10, 2020
@herecydev
Copy link
Contributor

Couldn't you push that logic into node creation, so that you're querying "pre-formatted" words? Do you have a concrete example?

@ghassanmas
Copy link
Author

@herecydev could you elaborate more please, I didn't quite get your question...

@herecydev
Copy link
Contributor

Well if you need something only during building, could you not do that work when creating graphql nodes and then just query the output of that? Then your react code doesn't need to rely on @formatjs and it won't be bundled

@ascorbic ascorbic added topic: webpack/babel Webpack or babel and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jun 16, 2020
@ascorbic
Copy link
Contributor

I may be missing something, but the problem I see here is that any package used in HTML generation is going to be needed when navigating between pages, as the page is generated dynamically then using the same code. It's also likely to be needed at rehydration time. Can you describe a case when this wouldn't be the case? Off the top of my head, you could in theory use an onCreateWebpackConfig hook to replace the loader for that module with the null loader if it's in the build-javascript phase. Not sure if that would work though.

@ascorbic ascorbic added the status: awaiting author response Additional information has been requested from the author label Jun 16, 2020
@ghassanmas
Copy link
Author

@ascorbic @herecydev I certainly need the packages to be used in the rehydration time (SSR). So telling the webpack to ignore loading the modules/packages would throw an error.

The thing is I need gatsby to use these packages in the rehydration time only. after that, the packages are irrelevant to be used/served to the client in the browser runtime environment.

@ascorbic
Copy link
Contributor

OK, but they need to be served to the browser in order to run at hydration time, so surely it's a moot point. They'll already be loaded and cached. If they're used on more than one page they'll be in a separate chunk so won't need loading again.

@ghassanmas
Copy link
Author

@ascorbic correct if I am wrong please
As to my understanding, the hydration occurs when gatsby is building/generating the HTML pages.
So I don't get it your point that:

they need to be served to the browser in order to run at hydration time.

Can you elaborate more please, and thanks for you patience.

@ascorbic
Copy link
Contributor

Hi. No, the SSR happens at build time, but hydration happens after the page is loaded. @joshwcomeau did a great post explaining it.

@ghassanmas
Copy link
Author

ghassanmas commented Jun 16, 2020

@ascorbic I stand corrected then. The packages/modules are needed only in the SSR process.
I just checked the source content of the HTML generated in the build phase. And it seems the module did its job in the SSR phase, so serving it to the client is irrelevant.

@ascorbic
Copy link
Contributor

OK, but as part of hydration, React needs to be able to generate the whole page again on the client side. Surely that means it will need those packages. Also when you navigate between pages it doesn't load the SSRd HTML, but rather just the page data, and renders it on the client. The scenarios where there will be packages used during SSR that are not needed during hydration are very rare.

@ghassanmas
Copy link
Author

@ascorbic

but as part of hydration, React needs to be able to generate the whole page again on the client side. Surely that means it will need those packages

Are you saying then ReactDOM hydration is as computationally expensive as ReactDOM render?...
When I read the article you referenced above as well as react docs that wasn't the case to my understanding... it seemed that react hydration job is mainly to add attach the events to the window objects...

Also when you navigate between pages it doesn't load the SSRd HTML, but rather just the page data, and renders it on the client.

It doesn't load the HTML it at all? Can you refer me to gatsby doc where it talks about it.
On the other hand if that is true, doesn't it downgrade the main value of gatsby pre generating the HTML pages

The scenarios where there will be packages used during SSR that are not needed during hydration are very rare.

while may not 💯 true for my personal case (I still need to do deeper analysis and debugging to confirm)., however I still believe its gotta be an essential feature to have for gatsby, given it affect the lighthouse score as mentioned above #24332 .

@wardpeet wardpeet removed the status: awaiting author response Additional information has been requested from the author label Jun 24, 2020
@ascorbic
Copy link
Contributor

No, it's not as expensive because it doesn't need to update the DOM apart from attaching listeners. That means it's a lot cheaper. The SSR is to improve the first page load. On subsequent navigation, by just loading the page data we can load far less data and it's quicker still.

I'm going to close this because I can't see a use case for it. The reality of React SSR is that you'll almost always need the same imports.

@ghassanmas
Copy link
Author

ghassanmas commented Jul 18, 2020

@ascorbic
Sorry I didn't got notified earlier about your last comments.
I have submitted a use case in this issue's description. e.g. when using the formatjs.

The reality of React SSR is that you'll almost always need the same imports.

On the other hand having you said "Almost" implies that are cases that where imports are not needed in the client as well.
Please reopen this issue to keep the discussion going about this important topic!.

@ghassanmas
Copy link
Author

There is also a very equivalent to a serious issue taken by next js' development team.
Check this article https://arunoda.me/blog/ssr-and-server-only-modules and this issue

@herecydev
Copy link
Contributor

I'm not sure comparing next.js issues is helping the issue here. Do you have an example (or can put together one) where you have something that is only used for SSR and shouldn't be included in a browser bundle? Otherwise we're just dealing in hypotheticals

@ghassanmas
Copy link
Author

ghassanmas commented Jul 19, 2020

Again, let put my example in details,
When using a i18n library where you need to insert only the values at render time, not event listener are needed.
Why I would need to use the module in the client side if the module doesn't attach event listeners or has no side effect relative user interaction. Only needed to when for doing the first render.

Next.js is very similar Gatsby, both projects can learn from each other. This is the spirit of open source.

@herecydev
Copy link
Contributor

When using a i18n library where you need to insert only the values at render time, not event listener are needed.
Why I would need to use the module in the client side if the module doesn't attach event listeners or has no side effect relative user interaction. Only needed to when for doing the first render.

Can you put together an actual reproduction (code) of that problem? React's hydration process mandates that they are the same thing:

React expects that the rendered content is identical between the server and the client. It can patch up differences in text content, but you should treat mismatches as bugs and fix them

If you can create a small reproduction that we can then use as a reference that'll be very helpful

@ghassanmas
Copy link
Author

@herecydev sounds fair, I will create a small gatsby project and link it here then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: webpack/babel Webpack or babel type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.
Projects
None yet
Development

No branches or pull requests

4 participants