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

gatsby's error catching development code found in production #23550

Closed
pindjur opened this issue Apr 28, 2020 · 4 comments
Closed

gatsby's error catching development code found in production #23550

pindjur opened this issue Apr 28, 2020 · 4 comments
Assignees
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) type: bug An issue or pull request relating to a bug in Gatsby

Comments

@pindjur
Copy link
Contributor

pindjur commented Apr 28, 2020

I found bunch of error checks and messages in gatsby production build coming from app-[hash].js
Can we remove this like React did in their production code?
I tried to contribute but im still begginer.
If this is a feature, can it be disabled through env variables?
We dont debug production code but if we do could it will be possible to debug via source maps without error checks in app-[hash].js?

@LekoArts LekoArts added the status: needs more info Needs triaging and reproducible examples or more information to be resolved label Apr 28, 2020
@LekoArts
Copy link
Contributor

Hi, please follow our issue template as otherwise we don't understand what you're asking and thus we can't help you.

<!--
  Please fill out each section below, otherwise, your issue will be closed. This info allows Gatsby maintainers to diagnose (and fix!) your issue as quickly as possible.

  Useful Links:
  - Documentation: https://www.gatsbyjs.org/docs/
  - How to File an Issue: https://www.gatsbyjs.org/contributing/how-to-file-an-issue/

  Before opening a new issue, please search existing issues: https://github.com/gatsbyjs/gatsby/issues
-->

## Description

Describe the issue that you're seeing.

### Steps to reproduce

Clear steps describing how to reproduce the issue. Please please please link to a demo project if possible, this makes your issue _much_ easier to diagnose (seriously).

How to Make a Minimal Reproduction: https://www.gatsbyjs.org/contributing/how-to-make-a-reproducible-test-case/

### Expected result

What should happen?

### Actual result

What happened.

### Environment

Run `gatsby info --clipboard` in your project directory and paste the output here.

@pindjur
Copy link
Contributor Author

pindjur commented Apr 28, 2020

Description

There is unnecessary code in production build generated by gatsby

Steps to reproduce

run gatsby build and gastby serve in gatsby starter default project root folder.
Open developer tools and go to sources panel.
open app.js
here is hosted gatsby starter default script at the time of writing
https://gatsby-starter-default-demo.netlify.app/app-812755817ebf00dbab64.js

Expected result

There should'n be error throwing and checking for error throwing

Actual result

there is 50 + error messages, even for PropTypes which is used only in development.
one of them are:
"var u=new Error("Calling PropTypes validators directly is not supported by the prop-types package. Use PropTypes.checkPropTypes() to call them. Read more at http://fb.me/use-check-prop-types");"

""

@pindjur pindjur changed the title gatsby's development code found in production gatsby's error catching development code found in production Apr 28, 2020
@madalynrose madalynrose added type: question or discussion Issue discussing or asking a question about Gatsby and removed status: needs more info Needs triaging and reproducible examples or more information to be resolved labels Apr 28, 2020
@pieh pieh self-assigned this Apr 29, 2020
@pieh
Copy link
Contributor

pieh commented Apr 29, 2020

Ok few notes:

prop-types specific: we do use https://www.npmjs.com/package/babel-plugin-transform-react-remove-prop-types but it tuns out it doesn't handle some cases (including ones we pack by default):

Cases that it can't handle is when you set propTypes on component that is wrapped with React.forwardRef (like our gatsby-link). I would expect similar thing happening with React.memo (but not confirmed that yet). I will be creating reproduction and creating issue in its repository of plugin about this.

In any case - in production shimed version is used (which is pretty much just this https://github.com/facebook/prop-types/blob/master/factoryWithThrowingShims.js and this does include the warning you mentioned).

But let's make a list of thing that are in default starter bundles and see which ones we actually can fix:

Ones that actually originate from gatsby (and not from some dependencies) and we can drop:

  • if (isNaN(Number(query))) {
    throw new Error(`useStaticQuery was called with a string but expects to be called using \`graphql\`. Try this:
    import { useStaticQuery, graphql } from 'gatsby';
    useStaticQuery(graphql\`${query}\`);
    `)
    }

    For this we can add process.env.NODE_ENV === `development` (similar to error we have a bit above it) to remove it from production (builds will fail anyway before we reach this stage)
  • } else {
    throw new Error(
    `The result of this StaticQuery could not be fetched.\n\n` +
    `This is likely a bug in Gatsby and if refreshing the page does not fix it, ` +
    `please open an issue in https://github.com/gatsbyjs/gatsby/issues`
    )
    }

    similarly - we can wrap it as well as we don't actually use this function in production (it get's replaced)

We for sure can't really touch react-dom ones:

  • Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings. for example

There few coming from @reach/router ( https://github.com/reach/router/blob/1f26ef04fce873a78c6a4c4605b6c1ec29bea7a8/src/index.js#L526-L578 )

Few coming from react-side-effect (dependency of react-helmet) - https://github.com/gaearon/react-side-effect/blob/f1e020719f5cc429b96b260cab80ed51b63421a5/src/index.js#L14-L22

There are few cases of error we throw, but they are actually meant specifically for production code in lot of cases as well.

@LekoArts LekoArts added type: bug An issue or pull request relating to a bug in Gatsby and removed type: question or discussion Issue discussing or asking a question about Gatsby labels May 26, 2020
@wardpeet wardpeet added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) and removed topic: StaticQuery labels Jun 24, 2020
@LekoArts
Copy link
Contributor

Closing this as stale since in the meantime Gatsby v4 and updated related packages were released. Please try with the latest versions and if you still see this problem open a new bug report (it must include a minimal reproduction).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

6 participants