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

refactor: add more params for better scalability #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yes1am
Copy link

@yes1am yes1am commented Jan 24, 2021

docz issue: doczjs/docz#1382

Thank you very much for your gatsby-plugin-svgr

I'm using docz and found that the current configuration doesn't support .mdx files, So I rewrote part of the code to support docz.

the default params is:

module.exports = {
  plugins: [
    {
      resolve: 'gatsby-plugin-svgr',
      options: {
        nonJsIssuerTest: /\.(?!(js|jsx|ts|tsx)$)([^.]+$)/,
        svgrIssuerTest: /\.(js|jsx|ts|tsx)$/,
        preGatsbyConfigTest: /\.(ico|svg|jpg|jpeg|png|gif|webp|avif)(\?.*)?$/,
        curGatsbyConfigTest: /\.(ico|jpg|jpeg|png|gif|webp|avif)(\?.*)?$/
      },
    },
  ],
}

I think the default params is compatible with both Gatsby < 2.3.0 and Gatsby ≥ 2.3.0 , because ico|svg|jpg|jpeg|png|gif|webp|avif includes ico|jpg|jpeg|png|gif|webp,but not very sure.

The following parameters make it possible to work properly in docz:

module.exports = {
  plugins: [
    {
      resolve: 'gatsby-plugin-svgr',
      options: {
        nonJsIssuerTest: /\.(?!(js|jsx|ts|tsx|mdx)$)([^.]+$)/,
        svgrIssuerTest: /\.(js|jsx|ts|tsx|mdx)$/,
      },
    },
  ],
}

Also, If gatsby's rule.test changes later, we can adjust the params to support.

If this Pull Request is passed, maybe we should also update the README, my English is not very good, I hope you can help to update it.

@oddui
Copy link

oddui commented Apr 15, 2021

Hello, @coreyward is there any chance to get this merged? It'll be good to be able to import svg components in mdx.

@coreyward
Copy link
Collaborator

@yes1am I like the more flexible approach of making the tests configurable, but the option names are pretty confusing. If you can refactor to make it clearer what's going on and add some documentation I think we can get this merged. It may also be worthwhile to look at #51 for Gatsby v3 support.

…nonJsRuleIssuer, and svgrRuleIssuer options

* Update README for the new options
@yes1am
Copy link
Author

yes1am commented Apr 16, 2021

Hi @coreyward, after check the #51, I've tried to refactor the code and add the relevant documentation. Since my English is not very professional, maybe you need to spend some effort to help revise, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants