glimmerjs / glimmer-blueprint

[MOVED] This package is now part of the Glimmer.js monorepo
https://github.com/glimmerjs/glimmer.js
25 stars 13 forks source link

Default to ember build -e production in package.json #12

Closed addyosmani closed 7 years ago

addyosmani commented 7 years ago

When scaffolding out the Glimmer blueprint for the first time, I assumed (after watching the EmberConf keynote) that the overall size improvements would be transparently visible to me. Instead I got a 1.8MB (pre-gzipped) bundle which was a little surprising.

Someone pointed out that I needed to run ember build -e production to strip out the dev assertions and after that I was able to get the advertised ~30KB build. Unfortunately, I think a lot of folks (in particular beginners) aren't going to consider the impact of environment variables on distribution sizes. Would it be out of the question to default ember build in the package.json below to include the production flag?

https://github.com/glimmerjs/glimmer-blueprint/blob/master/files/package.json#L10

Side: we're running into the "how do you avoid DEV mode being shipped to production" conversation with every major framework atm so this is definitely not an Ember specific issue. Hope y'all don't feel like I'm singling you out or anything. Great work on Glimmer!

rtablada commented 7 years ago

From some historic perspective ember build --watch is the command that is run under the hood when you run ember serve. Not so say that is particularly an excuse.

But this also means that there would be different environments for test, build, and serve

dgeb commented 7 years ago

@addyosmani Thanks for sharing your thoughts. I agree with your assessment that a production build target would be the least surprising default when running npm run build. This is probably the most accessible command for creating a distribution, and should require no special knowledge of ember-cli flags to just get started.

rtablada commented 7 years ago

@dgeb that's a good point, the build script in blueprints should have -e production set. That way it doesn't require knowing the CLI flags nor a change in defaults.

rwjblue commented 7 years ago

I think that we should do a better job of education. Explaining how it is important to have both development and production modes.

My suggestion is to remove the npm run build script and instead add npm run build:debug and npm run build:prod. Folks will not be confused (since they have to choose which build they want explicitly), and it gives us an opportunity to help folks learn why it is important...

dgeb commented 7 years ago

@rwjblue I like the idea of having separate npm tasks 👍

I guess it comes down to whether we additionally keep a default build task (if so, I would favor it being the same as build:prod).

rwjblue commented 7 years ago

To me, not having a (confusing) build task is partially the point/goal. We could easily have a build task that links to a good explanation of why we have two build types and help them understand which is the correct one (but not actually do a build)...

oskarrough commented 7 years ago

How about npm run build defaulting to production with an extra note about explicitly choosing build types? It's nice not having to introduce an ember-unique command to build the project when there's already yarn build.

Having npm run build do a production build out of the box also has advantages when it comes to integration with other services that rely on npm scripts (CI, hosting etc.)