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

Tina runs lots of code even in my website's production build #269

Closed
dwalkr opened this issue Oct 9, 2019 · 6 comments
Closed

Tina runs lots of code even in my website's production build #269

dwalkr opened this issue Oct 9, 2019 · 6 comments

Comments

@dwalkr
Copy link
Contributor

@dwalkr dwalkr commented Oct 9, 2019

Tina, you fat lard

Description

Short version: #268 occurs even in the website's production build.

The Tina sidebar is set to be hidden when building the prod site. Since the editing sidebar is not accessible, we should avoid running any Tina code since it's just wasted electricity.

It's worth noting that, semantically, hiding the sidebar doesn't really communicate that Tina code will be removed from the production build. Other solutions that involve conditionally running Tina but require a different configuration are welcome.

Steps to reproduce

  1. Create a Gatsby site and add Tina to it
  2. In gatsby-config.js, set the sidebar to be hidden on production like this:
{
  resolve: "@tinacms/gatsby-plugin-tinacms",
  options: {
    sidebar: {
      hidden: process.env.NODE_ENV === "production",
      position: "fixed",
    },
  },
},

Expected result

Tina does not run on production

Actual result

The Tina sidebar is inaccessible and content can't be edited, but Tina code still impacts page performance

@ncphillips

This comment has been minimized.

Copy link
Contributor

@ncphillips ncphillips commented Oct 9, 2019

One easy option would be to skip out if process.env.NODE_ENV === "production". My major concern with this approach is that maybe we want Tina to run in production. Maybe another flag would be better? e.g.

TINA_DISABLED=true gatsby build
@dwalkr

This comment has been minimized.

Copy link
Contributor Author

@dwalkr dwalkr commented Oct 9, 2019

I'm good with controlling via env var, and agree we should introduce our own (the value of which could default to NODE_ENV === 'production')

@ncphillips

This comment has been minimized.

Copy link
Contributor

@ncphillips ncphillips commented Oct 9, 2019

My only concern with our own variable, is I'm not sure it's as easy as TINA_DISABLED=true gatsby build I'm pretty sure that would require custom code. You need to manually add environment variables to process.env with webpack.

@ncphillips

This comment has been minimized.

Copy link
Contributor

@ncphillips ncphillips commented Oct 9, 2019

What if it was like:

if (DISABLED) {
  // return early
}

where our build replaces DISABLED with:

process.env.NODE_ENV === "production"

Later on, it could be replaced with:

process.env.NODE_ENV === "production" && !process.env.TINA_ENABLED

This would make it so tina in production is an opt-in feature, not an opt-out feature

Edit: this alone would prevent Tina from running. It would be up to the developer's build process to strip out the dead code. I believe CRA and Gatsby do this automatically.

@dwalkr

This comment has been minimized.

Copy link
Contributor Author

@dwalkr dwalkr commented Oct 9, 2019

My only concern with our own variable, is I'm not sure it's as easy as TINA_DISABLED=true gatsby build I'm pretty sure that would require custom code. You need to manually add environment variables to process.env with webpack.

At least in the case of Gatsby, we can programmatically edit webpack config from gatsby-plugin-tinacms

@ncphillips

This comment has been minimized.

Copy link
Contributor

@ncphillips ncphillips commented Oct 14, 2019

Because of the improvements made in #276, I am closing this issue in favor of the more specific #298

@ncphillips ncphillips closed this Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.