Skip to content
This repository has been archived by the owner on Nov 3, 2019. It is now read-only.

globalScope feature feedback #226

Closed
ChristopherBiscardi opened this issue Jan 4, 2019 · 21 comments
Closed

globalScope feature feedback #226

ChristopherBiscardi opened this issue Jan 4, 2019 · 21 comments
Labels
help wanted Extra attention is needed

Comments

@ChristopherBiscardi
Copy link
Owner

A couple ideas have surfaced in #138 so I'm creating a new issue to garner feedback around the new globalScope feature and how the API should evolve in the future.

example: A components folder for gatsby-mdx.

@ChristopherBiscardi
Copy link
Owner Author

@jlengstorf has some thoughts on a possible API.

globalScope: {
  Button: './src/components/button.js',
  Picker: {
    path: 'react-color',
    name: '{ SketchPicker as Picker }'
  },
}

@johno
Copy link
Collaborator

johno commented Jan 4, 2019

I kinda like the idea of a special mdx-components directory or file, tbh. It'd allow you to handle different import/export cases without having to learn new syntax or config keys.

// ./src/components/_mdx-components.js
import { SketchPicker as Picker } from 'react-color'
import * as Foo from 'bar'
import MUIButton from '@material/core/button'

Foo.Baz = () => <h1>baz</h1>

export { Foo, MUIButton }
export { default as Button } from './src/component/button.js'

@jlengstorf
Copy link
Contributor

I'm pretty into a special file, too. The tricky part, I guess, would be that if we actually ran it as code, we'd end up with something like this:

import * as GlobalScope from './src/componets/_mdx-components.js';

// How do we make `Picker` resolve from `GlobalScope.Picker`?

Maybe there's an easy way around that. A wrapper, maybe?

@jlengstorf
Copy link
Contributor

jlengstorf commented Jan 4, 2019

Also we'd need to handle a default export from _mdx-components.js. Is that just not allowed/ignored (probably easiest), or is there special handling?

@ChristopherBiscardi
Copy link
Owner Author

If we went with a single file, I'd say don't use the default export and just use export const for all components, then we can automatically check for MDX components (p, etc) and put them in MDXProvider in the wrapRootElement and leave the rest for MDXScopeProvider... but we could easily check to see if the default import was an object and merge it in with named exports or whatever. It's a non-issue IMO, just requires some edge-case handling code.

@ChristopherBiscardi
Copy link
Owner Author

also, pointing to a file doesn't really mix well with themes without some additional code that checks all the theme directories for the special file (but only if they have gatsby-mdx defined, etc)

@jlengstorf
Copy link
Contributor

Does that mean we'd have to do e.g.:

import { SketchPicker as Picker } from 'react-color';

export Picker;

?

Thinking about themes, I keep coming back to the idea of making it composable for e.g. plugins.

Like, gatsby-theme-blog exports something like PullQuote from node_modules/gatsby-theme-blog/components/_mdx-components.js, which we don't want to lose.

So being able to say something like adding a path that defaults to ./src/components/_mdx-components.js and auto-adds everything it exports (with overrides for collisions?) is super appealing to me.

Assuming that it would do something like this:

const finalGlobalScope = [
  ...globalScopeFromThemes,
  ...globalScopeFromPlugins, 
  ...globalScopeFromGatsbyConfig,
];

I assume that's easier said than done, but this would be an ideal outcome in my mind.

@ChristopherBiscardi ChristopherBiscardi added the help wanted Extra attention is needed label Jan 4, 2019
@rasovica
Copy link

rasovica commented Jan 8, 2019

I am having trouble getting this to work, I have version 0.3.3 of gatsby-mdx installed and the following in the config:

     {
      resolve: `gatsby-mdx`,
      options: {
          globalScope: `
            import Layout from '${__dirname}/src/components/layout/layout.jsx';
            export default {Layout};
          `,
      }

The import resolves without error but it is not found inside of .mdx.

## Test

<div><Layout /></div>

I would really like this to get to work as it would remove at least 10 lines of code from each mdx.

Am I doing something wrong? I can get rid of this error
error

@shripadk
Copy link

shripadk commented Jan 8, 2019

@rasovica You have to modify your gatsby-node.js file too and create a mdx-runtime.js file as explained in the kitchen-sink example. I modified it for my own project and this is what I have so far:

1. gatsby-node.js

/**
 * Implement Gatsby's Node APIs in this file.
 *
 * See: https://www.gatsbyjs.org/docs/node-apis/
 */

const path = require('path');

exports.createPages = ({ graphql, actions }) => {
  const { createPage } = actions;
  return new Promise((resolve, reject) => {
    resolve(
      graphql(`
        {
          allMdx {
            edges {
              node {
                id
                tableOfContents
                parent {
                  ... on File {
                    absolutePath
                    name
                    sourceInstanceName
                  }
                }
              }
            }
          }
        }
      `).then(result => {
        if (result.errors) {
          console.log(result.errors); // eslint-disable-line no-console
          reject(result.errors);
        }

        result.data.allMdx.edges.forEach(({ node }) => {
          const actualPath = node.parent.absolutePath
            .replace('.mdx', '')
            .replace(`${__dirname}/src/pages`, '');

          const options = {
            component: path.resolve('./src/gatsby-components/mdx-runtime.js'),
            context: {
              absPath: node.parent.absolutePath,
              tableOfContents: node.tableOfContents,
              id: node.id,
            },
          };

          createPage({
            path: actualPath,
            ...options,
          });
        });
      }),
    );
  });
};

exports.onCreateWebpackConfig = ({ actions }) => {
  actions.setWebpackConfig({
    resolve: {
      modules: [path.resolve(__dirname, 'src'), 'node_modules'],
    },
  });
};

2. mdx-runtime.js:

import React, { Component } from 'react';
import { graphql } from 'gatsby';

import MDXRenderer from 'gatsby-mdx/mdx-renderer';

// The layout that you were using earlier can be imported here
import { Layout } from './layout';

export default class MDXRuntime extends Component {
  render() {
    const { data, ...props } = this.props;

    return (
      <Layout {...props}>
        <MDXRenderer {...props}>{data.mdx.code.body}</MDXRenderer>
      </Layout>
    );
  }
}

export const pageQuery = graphql`
  query($id: String!) {
    mdx(id: { eq: $id }) {
      id
      code {
        body
      }
    }
  }
`;

The only issue currently with the globalScope feature is that you would need to restart the entire dev server for any changes/imports.

@ChristopherBiscardi
Copy link
Owner Author

@shripadk

You have to modify your gatsby-node.js file too and create a mdx-runtime.js file as explained in the kitchen-sink example.

You shouldn't have to do all that for globalScope to work.

@rasovica Do you mean that it isn't working for .mdx files in src/pages?

@rasovica
Copy link

rasovica commented Jan 9, 2019

@ChristopherBiscardi I was confused by @shripadk response as well because I followed #138 guide to adding components to the global scope.
Yes in a .mdx file in src/pages. I am pretty sure that I have a version of gatsby-mdx that has globalScope as I can console log from the code block and see that Layout component indeed gets imported

@shripadk
Copy link

shripadk commented Jan 9, 2019

@ChristopherBiscardi I tried it with a blank starter and it did not work as well. I'll put up the repo by end of the day for you to try.

@rasovica I followed the same guide as well. But it doesn't work and I got the same error as you did. So I followed the kitchen-sink example and realised that it only worked with modifications to gatsby-node.js and using a custom renderer.

This is my gatsby-config.js which you can verify for yourself:

module.exports = {
  siteMetadata: {
    title: 'Example',
  },
  plugins: [
    'gatsby-plugin-react-helmet',
    'gatsby-plugin-styled-components',
    {
      resolve: `gatsby-plugin-manifest`,
      options: {
        name: 'gatsby-starter-default',
        short_name: 'starter',
        start_url: '/',
        background_color: '#663399',
        theme_color: '#663399',
        display: 'minimal-ui',
        icon: 'src/images/Icon.svg', // This path is relative to the root of the site.
      },
    },
    'gatsby-plugin-offline',
    'gatsby-plugin-typescript',
    {
      resolve: 'gatsby-mdx',
      options: {
        globalScope: `
          import { SketchPicker } from 'react-color';
  
          export default {
            SketchPicker,
          };
        `,
      },
    },
    {
      resolve: 'gatsby-source-filesystem',
      options: {
        path: `${__dirname}/src/pages`,
      },
    },
  ],
};

As you can see it does use globalScope as described in #138.

Can you try it with the above modifications and see if it works? If I comment gatsby-node.js code and use a default layout it stops working.

@shripadk
Copy link

shripadk commented Jan 9, 2019

@ChristopherBiscardi I have created a repo here: https://github.com/shripadk/gatsby-mdx-test for you to try out.

Just used the default gatsby starter package with gatsby-mdx. The master branch is without the custom renderer and with-runtime branch is the one with the modifications. Please try it out and let me know if I am doing something wrong in the master branch.

The test page is located at src/pages/test.mdx and the location is http://localhost:8000/test

  1. Testing from master branch:

screenshot 2019-01-09 at 12 38 29 pm

  1. Testing from with-runtime branch:

screenshot 2019-01-09 at 12 39 58 pm

@ChristopherBiscardi
Copy link
Owner Author

@shripadk this issue is for feature API design feedback, not bug reports. Please open a new issue next time. Here's one for the issue you're both seeing right now: #239

@shripadk
Copy link

@ChristopherBiscardi I understand the difference. I wasn't aware that it was a bug in the first place. I thought that globalScope can only be used with modifications to gatsby-node.js and gatsby-config.js because I followed the kitchen-sink example. Only after your response did I realise that it was intended to be used without those modifications. So created and shared a repo to know if I was making any mistake in the config. Now that you say it's a bug i'll continue the discussion there. You can blame it on my excitement to try out the new API before it was stable 😄Sorry about that!

@ChristopherBiscardi
Copy link
Owner Author

glad you're excited about the feature :)

@ChristopherBiscardi
Copy link
Owner Author

Thanks for the feedback all, going to close this issue now as conversation has died down.

@alexandernanberg
Copy link
Collaborator

alexandernanberg commented Feb 28, 2019

@ChristopherBiscardi Would it work if we would just context for this? So you would do <MDXScopeProvider scope={components} /> in a layout/app component. Not a huge fan personally of the separate file or the globalScope options.

@ChristopherBiscardi
Copy link
Owner Author

@alexandernanberg I recently submitted a PR to MDX to support this functionality through an enhanced MDXProvider. It landed in the MDX 1.0 branch so will be published then and gatsby-mdx@1.0 will remove the globalScope option because it can be served by using MDXProvider in wrapRootElement.

@alexandernanberg
Copy link
Collaborator

Ah that’s fantastic, looking forward to try it out 😄

@rwieruch
Copy link

Difficult to find the latest material on "how to define global scoped components with Gatsby MDX" :) The final culprit was not using @mdx-js/react but still @mdx-js/tag.

If anyone is interested in seeing global components in action: https://github.com/rwieruch/gatsby-mdx-blog-starter-project

Thanks for your hard work @ChristopherBiscardi ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants