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

[1.0] App bundle not transpiled or minified #974

Closed
0x80 opened this issue May 16, 2017 · 14 comments · May be fixed by harunpehlivan/gatsby#471, philipedin/gatsby#509, tejzpr/gatsby#25, tejzpr/gatsby#51 or tejzpr/gatsby#66

Comments

@0x80
Copy link
Contributor

0x80 commented May 16, 2017

It looks like the result of gatsby build is not minified or transpiled fully. While testing IE11 on browserstack it can't load the script because there are still backtick characters.

If I try to load https://gatsbygram.gatsbyjs.org/ it has the same issue.

The generated bundle contains this

/***/ "./node_modules/gatsby-plugin-google-analytics/gatsby-browser.js":
/***/ (function(module, exports, __webpack_require__) {

	"use strict";
	
	exports.onRouteUpdate = function (location) {
	  // Don't track while developing.
	  if (("production") === `production`) {
	    ga(`set`, `page`, location.pathname);
	    ga(`send`, `pageview`);
	  }
	};

/***/ }),

Is there some setting I should tweak maybe? I would expect IE11 to be supported out of the box.

@KyleAMathews
Copy link
Contributor

Hmmm this is a babel settings problem. Looking at the different site's babelrc there's no consistency and at least one has the presets in the wrong order 😨

I've meant in the past to update how Gatsby handles babel settings so this is a good time to do it. Using the babel stages presets is discouraged now in favor of https://github.com/babel/babel-preset-env. We need to update the default babel settings to use that + document how to make common modifications e.g. supporting even older browsers.

Also our babel stuff needs to be Reduxified still.

Lemme know if you want to tackle this!

@0x80
Copy link
Contributor Author

0x80 commented May 16, 2017

I don't have a whole lot of time but this shouldn't be too difficult so I could probably pick this up tomorrow.

But what do you mean by Reduxified?

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 16, 2017 via email

@0x80
Copy link
Contributor Author

0x80 commented May 16, 2017

@KyleAMathews cool, I'll look into it tomorrow

@0x80
Copy link
Contributor Author

0x80 commented May 22, 2017

@KyleAMathews So "tomorrow" didn't really happen 🙂

I had another look but before I start working on this I think I need to understand the problem better and also the preferred solution.

From what I read, the gatsby core sets a number of babel plugins and presets. Then plugins can modify these via the api runner, and the optional user's babel config (file or package.json) is read and merged in. Then this resulting babel config is used to build the users website.

However the problem with the analytics plugin comes from the fact that it is transpiled when gatsby built its packages. The dist source from the analytics plugin contains backticks because the global babel env preset is currently targeted at node 4 and not browsers like ie11.

I can fix this by adding "browsers": ["> 1%", "last 2 versions"] to the gatsby babelrc, but that affects the build output of all gatsby packages, including the server side ones.

Ideally I guess all gatsby code should only be built for the current node target of the user, and then a plugin like analytics should transpile to whatever browser target the users specifies in its babel config, but I am not sure if this is possible or how to achieve that.

I think a minimal solution could be to give the client (browser) side packages their own babelrc with a full transpiled output for all browsers, but that doesn't let the user configure anything.

I wonder what you had in mind for the user-land api modifying the babelrc config, because I don't see how that would work for any of the packages.

@0x80
Copy link
Contributor Author

0x80 commented May 23, 2017

For the analytics transpiling issue I've now added a babelrc to the plugin, since I noticed the same was done for gatsby-link #1020

@KyleAMathews
Copy link
Contributor

The babelrc API is for overriding the babelrc config for their own code not for core packages. It needs to be usable by users but also by plugin authors who need to add in custom babel plugins.

Core packages should compile down to a universal ES5 version so everyone can just use them so I think adding the custom .babelrc files to client packages is best for now. Ideally I think we'd setup our babel builds to automatically use a different babel config for gatsby-browser.js files.

@0x80
Copy link
Contributor Author

0x80 commented May 23, 2017

@KyleAMathews with the analytics backticks out of the way I now get an error in IE about the use of Array find(). This is in production-app.js, so not an isolated client package. How would you suggest to tackle this?

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 23, 2017 via email

@KyleAMathews
Copy link
Contributor

Oh hmm nvm. Yeah we shouldn't use find in core code I guess.

@KyleAMathews
Copy link
Contributor

#1024

@KyleAMathews
Copy link
Contributor

No see my PR #1024 I'm just using array.some which has support in IE back to IE9.

@KyleAMathews
Copy link
Contributor

Thanks for the IE testing!

@0x80
Copy link
Contributor Author

0x80 commented May 23, 2017

Ah yes, ok. Thanks for fixing! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment