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

Imports in MDX pages go to the main bundle #23345

Closed
trustin opened this issue Apr 21, 2020 · 14 comments
Closed

Imports in MDX pages go to the main bundle #23345

trustin opened this issue Apr 21, 2020 · 14 comments
Labels
type: question or discussion Issue discussing or asking a question about Gatsby

Comments

@trustin
Copy link

trustin commented Apr 21, 2020

Summary

Any imports I put into an MDX page become a part of the main bundle (app-*.js). For example, the following JSON file and React component in an MDX file will be added to app-*.js:

import someData from './some-data.json';
import SomeComponent from '../../components/some-component';

I expect such imports to be tied to each page rather than to the main bundle, for optimal use of network traffic. This can't scale as the number of pages increases.

This comment by @pieh implies this is a known issue. Is it correct? If so, what's the best way to work around this issue?

Relevant information

N/A - I think summary explains the problem. Let me add some if requested.

Environment (if relevant)

  System:
    OS: Linux 5.4 Arch Linux
    CPU: (16) x64 AMD Ryzen 7 2700X Eight-Core Processor
    Shell: 5.0.16 - /bin/bash
  Binaries:
    Node: 10.20.1 - /tmp/yarn--1587472616909-0.021293460855316138/node
    Yarn: 1.22.4 - /tmp/yarn--1587472616909-0.021293460855316138/yarn
    npm: 6.14.4 - /usr/bin/npm
  Languages:
    Python: 3.8.2 - /usr/bin/python
  Browsers:
    Chrome: 81.0.4044.113
    Firefox: 75.0
  npmPackages:
    gatsby: ^2.19.45 => 2.20.27 
    gatsby-image: ^2.3.1 => 2.3.4 
    gatsby-link: ^2.2.31 => 2.3.4 
    gatsby-plugin-google-analytics: ^2.2.0 => 2.2.4 
    gatsby-plugin-import: ^2.1.5 => 2.1.5 
    gatsby-plugin-less: ^3.0.21 => 3.1.3 
    gatsby-plugin-manifest: ^2.3.3 => 2.3.5 
    gatsby-plugin-mdx: ^1.1.4 => 1.1.9 
    gatsby-plugin-react-helmet: ^3.1.24 => 3.2.4 
    gatsby-plugin-sharp: ^2.5.3 => 2.5.6 
    gatsby-plugin-sitemap: ^2.3.1 => 2.3.5 
    gatsby-plugin-typescript: ^2.2.5 => 2.3.3 
    gatsby-remark-autolink-headers: ^2.2.1 => 2.2.3 
    gatsby-remark-draw: ^1.0.16 => 1.0.16 
    gatsby-remark-images: ^3.2.2 => 3.2.4 
    gatsby-source-filesystem: ^2.2.2 => 2.2.4 
    gatsby-transformer-sharp: ^2.4.2 => 2.4.6 

File contents (if changed)

gatsby-config.js: N/A
package.json: N/A
gatsby-node.js: N/A
gatsby-browser.js: N/A
gatsby-ssr.js: N/A

@trustin trustin added the type: question or discussion Issue discussing or asking a question about Gatsby label Apr 21, 2020
@trustin
Copy link
Author

trustin commented Apr 21, 2020

Just in case it makes any difference, all the MDX files are located in src/pages directory.

@pieh
Copy link
Contributor

pieh commented Apr 21, 2020

The "best" workaround for now is to wrap imported components with React.lazy / react-loadable. The cons of that is the components will not render for .html and there will be "flash" of content first before components loads and appear on the page

Yes, it is known issue and there is work being done on this, but it's pretty far way from doing it nicely (for now it's discovery, I had proof of concept working but it was very limited). So for steps we need in gatsby core to make this happen:

  • detach <StaticQuery> / useStaticQuery result from webpack bundles (so they use same pipeline as page queries). Difficulties - we will need to track which pages exactly do need static queries results (so this includes analyzing webpack's dependency graph) and we need to add metadata to page-data and app-data files that list required static query result for app / pages
  • Add gatsby core APIs for users/plugins to be able to register additional components to be handled by webpack and consume them in runtime/SSR (gatsby-plugin-mdx need to resort to hack right now that results in those being bundled in app chunk). For consuming them, the runtime resource loader need to be able to handle that
  • adjust gatsby-plugin-mdx to make use of new APIs

@trustin
Copy link
Author

trustin commented Apr 21, 2020

Thanks a lot for a super quick response. What workaround would you suggest for imported JSON? Should I move the page out of 'src/pages', load the JSON in 'gatsby-node.js' instead of importing in '.mdx', and call 'createPage()' with the loaded JSON?

@pieh
Copy link
Contributor

pieh commented Apr 21, 2020

Ah, I didn't notice that you import JSON there. I don't know to be honest how to best approach this yet other than "hiding" this in extra component:

-import someData from './some-data.json';
-import SomeComponent from '../../components/some-component';
+import SomeWhatRedundantComponentThatHidesImportsAndMakeThemLazy from './lazy-thing'

and then lazy-thing.js could be

import Loadable from 'react-loadable';

const LoadableComponent = Loadable({
  loader: {
    Component: () => import('../../components/some-component'),
    Data: () => import('./some-data.json'),
  },
  render(loaded, props) {
    let Component = loaded.Component.default
    let data = loaded.Data
    return <Component someProp={data} />
  }
})

export default LoadableComponent

(that's using Multiple resources case for react-loadable - https://github.com/jamiebuilds/react-loadable#loading-multiple-resources )

@trustin
Copy link
Author

trustin commented Apr 21, 2020

Wouldn't it make the page unfriendly to search engines? Would my idea of gatsby-node.js hack work? 🤔

@pieh
Copy link
Contributor

pieh commented Apr 21, 2020

Oh, I found out we should be able to do this directly in .md(x) file using similar technique as described in https://johno.com/react-hooks-in-mdx/ (so you wouldn't need separate wrapper files for it, and just use react-loadable directly in .md files)

@pieh
Copy link
Contributor

pieh commented Apr 21, 2020

Wouldn't it make the page unfriendly to search engines? Would my idea of gatsby-node.js hack work? 🤔

Potentially yes, but crawlers are pretty good at running javascript code nowadays. The gatsby-node.js idea would work for sure too

@trustin
Copy link
Author

trustin commented Apr 21, 2020

Thanks a lot for the suggestions. Let me give them tries and report back.

Should we turn this issue into a bug report or enhancement request? I can't find an issue filed for this yet.

@trustin
Copy link
Author

trustin commented Apr 22, 2020

I worked around this problem successfully by using @loadable/component:

import loadable from '@loadable/component';

export const ArticlesProvider = loadable.lib(() => import('./articles.json'));

export const ArticleList = loadable(() => import('../../components/article-list'));

# Articles and slides

<ArticlesProvider>
  {({ default: articles }) => <ArticleList dataSource={articles} />}
</ArticlesProvider>

It was slightly easier to load multiple modules using react-loadable, but ended up with @loadable/component since it seems to be actively maintained.

@trustin
Copy link
Author

trustin commented Apr 22, 2020

Another workaround that works when you want to load only a JSON file into an MDX page (gatsby-node.js): i.e. no component lazy-loading

// Loads the '.json' file next to the page into 'pageContext.companion'.
// Note that this will cause some GraphQL schema warnings while building,
// but it seems safe to ignore.
exports.onCreatePage = ({ page }) => {
  const componentPath = page.componentPath;
  if (!componentPath) {
    return;
  }

  const ext = path.extname(componentPath);
  const jsonPath = `${componentPath.substring(
    0,
    componentPath.length - ext.length,
  )}.json`;

  if (fs.existsSync(jsonPath)) {
    /* eslint-disable global-require */
    /* eslint-disable import/no-dynamic-require */
    // eslint-disable-next-line no-param-reassign
    page.context.companion = require(jsonPath);
    /* eslint-enable import/no-dynamic-require */
    /* eslint-enable global-require */
  }
};

For example, in articles.mdx, you can access the content of articles.json via props.pageContext.companion:

<ArticleList dataSource={props.pageContext.companion} />;

@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label May 12, 2020
@CanRau CanRau added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels May 16, 2020
@LekoArts LekoArts removed the not stale label Jun 3, 2020
@LekoArts
Copy link
Contributor

LekoArts commented Jun 3, 2020

Since the original question was answered, I'll close this issue.
@pieh is working on a general approach that will also solve this problem. Please look out for new features which we'll announce then.

@LekoArts LekoArts closed this as completed Jun 3, 2020
@i-bsd
Copy link
Contributor

i-bsd commented Mar 16, 2021

Has anything changed on this since this issue was raised last year?

I'm having the same problem. Large site, MDX, wide use of heavy MDX imports, all included in the main app bundle. Seriously hurting my performance.

Is @loadable/component still the only solution here or has there been progress since?

@JoseOcampoDw
Copy link

same, any updates here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question or discussion Issue discussing or asking a question about Gatsby
Projects
None yet
Development

No branches or pull requests

8 participants