Explore encouraging users to not ship DEV mode to production

[addyosmani]

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
Developers meaning to do the right thing will often accidentally ship DEV mode to production rather than PROD mode. This can have a significant impact on performance. Although DEV->PROD is a one line change, it's something React could explore encouraging.

There's great nuance here and I know that there's balance to be struck between the overall DX value this brings vs UX. Another challenge is that the change itself is trivial to make. It's unclear whether the right solution here is better defaults or stronger advocacy. Folks like @sebmarkbage have been acknowledging that this is a known issue so perhaps there's room for discussion to help improve this.

He's also noted that a switch from no warnings to DEV may require some folks to fix whole codebases which is also suboptimal. There may be an in-between solution worth talking about here however.

What is the expected behavior?

React encourages users to ship PROD mode to production rather than DEV. I would be open to a solution that is either provided at the library layer (or somehow tackled during build/bundling time by Webpack) that tries to ameliorate this.

This thread had a number of suggestions ranging from localhost detection, to alerts to injecting 'dev mode' messages to the DOM if used in a production environment. Something like this:

Alternatively, @TheLarkInn was proposing that we tried to standardize on ENV configs being required to better facilitate detection of messaging like this. It's unclear which of these would be the most realistic. There are likely other ideas React core might have around how to tackle the problem.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

All recent versions.

This thread from @jordwalke prompted this issue. I think he also makes a fair point regarding benchmarks, but I care about how we can help folks ship the prod experience y'all have worked on optimizing to end customers in all it's glory.

[aweary]

For reference: #8782

[sebmarkbage]

For context we already warn if we detect that you've minified a DEV version of React:

react/src/renderers/dom/ReactDOM.js

Lines 91 to 98 in 8791325

	var testFunc = function testFn() {};
	warning(
	(testFunc.name || testFunc.toString()).indexOf('testFn') !== -1,
	'It looks like you\'re using a minified copy of the development build ' +
	'of React. When deploying React apps to production, make sure to use ' +
	'the production build which skips development warnings and is faster. ' +
	'See https://fb.me/react-minification for more details.'
	);

As far as we can find similar heuristics to notify users, and maybe even more aggressively pop up a DOM dialog, we should.

[sebmarkbage]

I also want to make it clear the warnings we provide can significantly improve performance if people pay attention to it. This thread explains some rationale for why it is difficult to deploy this after the fact if it is not the default.

[aickin]

I'd also like to chime in with a suggestion of a single console.warn if renderToString is called in dev mode. Obviously, in most situations renderToString is called in node, where we can't alert or pop up a DOM dialog.

Unfortunately, I'd really like to be able to detect not just if NODE_ENV is set to production, but also if process.env.NODE_ENV has been compiled out. Does anyone know of a way to do that?

[addyosmani]

Thanks for the thread @sebmarkbage and I acknowledge the difficulties of deploying after the fact. I'm also appreciative of the current warnings. It appears that some developers may not check the console output of their deployed sites as often as they could. It's a good first step however.

As far as we can find similar heuristics to notify users, and maybe even more aggressively pop up a DOM dialog, we should.

I'd be grateful for improvements to the heuristics used to notify users. A more aggressive DOM dialog would go a long way in helping. It would ensure the site continued working for end-users but provides an active hint that there are low-hanging perf fruit developers can pick.

The alternative is that we find a way to fix this at a build-tool / ENV level as mentioned in the original post. That would avoid any DOM-injection being necessary.

[aweary]

Injecting any messaging into the DOM seems dangerous and a little too assuming. That opens up the possibility of end users getting unexpected and confusing alerts which seems unacceptable IMO.

@TheLarkInn was proposing that we tried to standardize on ENV configs being required to better facilitate detection of messaging like this

This feels like the ideal space to address this. It's a build issue and should be, if possible, addressed by the build tools.

[surma]

Anecdotal: Warnings in the console have been gone unseen (or ignored) in the past. I have no hard numbers here, but I think console-based warnings are not enough. I agree with @addyosmani that a DOM-based warning would go a long way.

[aweary]

@surma maybe they should use console.warn or console.error for better visibility 😉

I don't see how it would be acceptable in any scenario to inject content into an application only when it's running in production. You're making a huge assumption that the message would be caught before the application was pushed to actual users, where the message could potentially hurt UX in a major way.

[sebmarkbage]

Btw, we're going to be add more comprehensive error handling support in Fiber so that you can replace components that have failures (thrown errors) with custom error views. Even in the default scenario we're likely going to be very aggressive, and just remove that component from the tree if it errors so you're really going to want to have a custom error UI for your users anyway.

We might be able to trigger such an error for this warning.

[surma]

I honestly don’t think that console.{error,warn} over console.log would have changed anything. As I said, that story is anecdotal.

I am also not saying that showing a DOM dialog is the solution. It’s something I’d personally be comfortable with, though. If staying in dev mode has negative impact, at least users would know that something is wrong and probably starts hitting the “Help” button or something.

Bottom line: I think frameworks need to get in the developer’s face at some point. I’d love to brainstorm on this to see what steps and compromises framework authors are willing to take to prevent people from deploying in dev mode in the future.

[acdlite]

What if React just doesn't work at all unless you provide an environment, regardless of whether it's development or prod? That way there's a conscious choice being made one way or the other.

[jide]

About the message injected in the DOM, it could be disabled using a global or something. No big deal IMO. If you disable it, you kind of acknowledge you know what you're doing.

Problem with a console message is that sometimes people log a lot of things, or have other warnings they ignore, and it's easy to not see that first console message past the scroll...

[jide]

With a mandatory env, inevitably boilerplates etc. will set the env var so you can just start using it I'm afraid :/

[aweary]

I honestly don’t think that console.{error,warn} over console.log would have changed anything.

Do you think its a problem with developers just not checking the console, or the console being overloaded with warnings? Could this be (partially) addressed with a more general approach regarding developer education?

I am also not saying that showing a DOM dialog is the solution. It’s something I’d personally be comfortable with, though. If staying in dev mode has negative impact, at least users would know that something is wrong and probably starts hitting the “Help” button or something.

I understand that, but I'm just saying I don't think its a solution let alone the solution. Its good that you're comfortable with it, but I think it's better to err on the side of caution and assume that most people don't want unexpected errors displayed to their users.

Bottom line: I think frameworks need to get in the developer’s face at some point. I’d love to brainstorm on this to see what steps and compromises framework authors are willing to take to prevent people from deploying in dev mode in the future.

I am 💯 for getting in the developer's face, but it's important to do it at the right place. To me, that's the build step, not production.

About the message injected in the DOM, it could be disabled using a global or something. No big deal IMO. If you disable it, you kind of acknowledge you know what you're doing.

Having it enabled by default is just as bad as not making it configurable: the default behavior could result in unexpected behavior for the end user. If anything it should be disabled by default, but then that defeats the entire purpose since developers could just fix the initial problem once they're aware of it.

Problem with a console message is that sometimes people log a lot of things, or have other warnings they ignore, and it's easy to not see that first console message past the scroll...

I totally get that, the console can get crowded and it's easy to miss stuff. But it's crowded for the exact reasons I'm arguing: it's the place the provide developers with feedback or errors. It's out of the way of the user experience, which injected messages aren't.

[jide]

Makes sense, I understand the reasonning.

Well maybe make the thing pop out using console formatting would be something at least.

[jide]

[gaearon]

The problem is almost nobody looks at console in production. You can use any font there and people won't notice it.

However if we make it show a message by default in production as a breaking change (in 16 or 17) then it would be hard to miss. I mean, it would happen the first time you would try to deploy it (for new users), and existing users should be reading release notes when updating to a new major. So I think it's doable as long as we are super explicit about it and it's impossible to miss the message.

[addyosmani]

The problem is almost nobody looks at console in production. You can use any font there and people won't notice it.

I can only comment on the Chrome team's experience with this, but I'd concur. Most folks will notice console messages during their iteration workflow but a far smaller % look at warnings on production sites and fewer act on them.

However if we make it show a message by default in production as a breaking change (in 16 or 17) then it would be hard to miss. I mean, it would happen the first time you would try to deploy it (for new users), and existing users should be reading release notes when updating to a new major. So I think it's doable as long as we are super explicit about it and it's impossible to miss the message.

Thanks for being open to a change like that @gaearon. What would it take to get agreement on trying for a message by default in a future release? 😄

[jakearchibald]

I agree that console warnings aren't the solution & a page-visible warning is much better.

The page-visible warning could:

• Alert the dev that the site is in dev mode
• Link to the docs on the benefits, and how to ship without it
• A link to disable the message for… I dunno 2 hours?

Disabling the message is important, as it may be interfering/covering something on the page. Since this setting would be stored in localstorage, the warning will still appear on the live server because it's a different origin.

Yeah, it's pretty horrible if real users see this message on live sites, but it feels like the kind of problem devs will be encouraged to fix, whereas some seem to be happy to live with the performance issues of dev mode.

[rtorr]

If the first time I saw that warning (the insert into DOM one), was in production, I would be fairly upset. The warnings need to happen ahead of time.

[jakearchibald]

@rtorr my suggestion is that it happens whenever the site is in dev mode, so it should be seen ahead of time unless I'm missing something?

[rtorr]

@jakearchibald sorry for the confusion, my reply was not directed at yours. I just want to point out to the thread that if we were to use the 'insert into the dom' solution, we should be very careful and make sure users know before they push a thing (some how, I have no good idea here).

I just see some dev forgetting a setting or something and management freaks out. Is that possibility worth it for the consequences of having dev mode in production?

[Pajn]

A DOM based warning that I have to constantly disable is not okay, it must be possible to disable it forever, and maybe it should never show at all for localhost.

A thing that just hit me if it would be possible to have some kind of a flag in the browser that you have to enable to activate the devtools (maybe a big overlay in them with "Are you a developer? [Yes/No]") that the page can detect and only show the warning for developers. Worded correctly it might help with self-XXS attacks as well.

[jakearchibald]

A DOM based warning that I have to constantly disable is not okay, , it must be possible to disable it forever

Sites launching with dev-mode on is also not okay. Maybe the message only needs to be dismissed once per day? But the longer the period, the more likely it is that'll it'll end up on live. If it can be disabled forever, we're right back where we started.

[rtorr]

I also don't think an unintended dom node in production is OK.

I think that either way, we will always have an edge case. If this problem happens all of the time, then maybe the delivery of dev mode is wrong. Although not ideal in a perfect world, but if we find prod mode this important that we are willing to modify someone's application, maybe it should be default and dev mode should be opt in.

[jakearchibald]

@rtorr

I also don't think an unintended dom node in production is OK.

Why? (I'm not saying you're wrong, would just like to hear your reasons)

[ropilz]

Maybe adding a setting to define prod Domain. If prod Domain is not set then we always get the warning about DEV mode (with a request to set Domain URL), if it is set then we only get warning when the URL matches with prod domain. We could even bind any service we want to notify devs

[addyosmani]

I'm glad there's constructive discussion here. There are two solutions here that I can see solving the problem. Webpack could force specifying NODE_ENV which React could then use to more easily avoid folks shipping DEV to PROD, but that would be a breaking change to Webpack. I'm talking to Sean now about how feasible something like that could be for Webpack 3. Keeping the React + Webpack stack beginner and perf friendly is something I know both camps care about.

The second (DOM injection idea) is something React could do and as Jake mentioned, balance the UX by allowing the message to be shown once a day or be dismissed. It's a one line change to fix the issue then you just have to redeploy. I completely empathise with not wanting management to freak out.

If we're to get more React sites shipping the far faster experience FB worked on to prod something might have to give. If anyone has better ideas please suggest them.

[rtorr]

@jakearchibald

Why? (I'm not saying you're wrong, would just like to hear your reasons)

Back to my comment above, unless we are able to let developers know ahead of time (which seems to be the actual problem to solve), I find it kind of extreme to devalue someones product by displaying a warning to developers on their production page. In a lot of cases this could potentially hurt the product more than the performance of dev mode.

No matter what we do, someone is going to ship whatever is default into production, why not make production default? Why not improve dev mode to the point where it's not that big of a impact?

[jakearchibald]

@rtorr https://twitter.com/sebmarkbage/status/820054247496503296

[gaearon]

While I agree about this being true about most warnings, React warnings specifically point out the bugs in your code. They are not just suggestions.

The dialog is easy to dismiss and the individual warnings are easy to snooze (in case you don't care about them for a while) but they need to be fixed.

For comparison, this is how the dialog looks in the Facebook codebase:

Thousands of engineers don’t have a problem with it and are more productive thanks to it, so we think it’s a reasonable default. To be clear, the open source version will be less shouting:

If you have suggestions about style tweaks please feel free to comment on #7360.

[bvaughn]

Adding onto what Dan said, I'm building on top of the #7360 to hook into our recently-added error logger flow. I'm currently trying out a couple of styles of toast notifications that are a little less intrusive. I'll post some screenshots and/or a Plnkr soon for feedback.

[taion]

Would these warnings include the "minified dev code" warning? That'd solve a lot of things quite neatly if so.

[bvaughn]

I don't see why it couldn't also be used for that purpose.

[KrisSiegel]

@gaearon

While I agree about this being true about most warnings, React warnings specifically point out the bugs in your code. They are not just suggestions.

Those should be errors or exceptions IMO. Why are they not? Exceptions force things to be corrected but a dismissible warning does not.

Thousands of engineers don’t have a problem with it and are more productive thanks to it, so we think it’s a reasonable default.

I mentioned this in a prior point but I would guess those engineers are likely better than a good 90% of people who will use the open source version. I find it the opposite of a reasonable default. There is a reason development tools have warnings; reinventing them doesn't make sense to me. It'll get disabled and never seen again.

This just looks like React is trying to do too much IMO.

---

Anyway, I'm just repeating my arguments that I've already said twice. If you're going to go ahead then feel free. In my opinion it's not a good business to get into. I'll just leave this short anecdote about a time when I wanted to punch some toast...

When I did government contracting we had a common library all front ends had to use. It would take errors in the console and display them as a toast pop up. No only did it get deployed to production multiple times by multiple teams but many developers saw it once then asked how to permanently disabled it. I see this as more of the same.

[addyosmani]

We’ve long considered enabling a “warning box” for React warnings in development. You can see a demo here (PR: #7360). It only shows up when you have warnings but they are very common during development (and should always be fixed), so presumably anyone who spent more than five minutes developing an app will see the dialog and be aware that it exists.

I really like #7360, @gaearon. It's heartening to hear support for highlighting the need to switch to PROD for deployments in the new warning box. It's nice and visual.

Adding onto what Dan said, I'm building on top of the #7360 to hook into our recently-added error logger flow. I'm currently trying out a couple of styles of toast notifications that are a little less intrusive. I'll post some screenshots and/or a Plnkr soon for feedback.

@bvaughn Looking forward to seeing more of your iterations :)

For folks who feel the warning box approach may be too intrusive, other libraries (e.g VueJS) already display DOM overlays during your dev/iteration workflow to encourage bug-fixing or slow-paths:

My own experience has been that while it's a minor inconvenience, these messages are more obvious than what you might see in the console. I feel Dan's right that it'll at least place more emphasis on dev mode not being something you should deploy to prod and will hopefully lead to more sites shipping "faster mode" to their end users.

After this change, it will be harder to be unaware of the development mode. You’ll likely search for “how to remove the warning dialog” and learn about building for production. If you don’t, you’re likely to get some warnings deployed at some point, and your users will see them. I think that in this case people won’t blame React per se because we don’t just show the box to be obnoxious—we just do what the development mode is supposed to.

[Pajn]

Those should be errors or exceptions IMO. Why are they not? Exceptions force things to be corrected but a dismissible warning does not.

While they should be fixed, I might have more urgent matters at hand. For example do I often prototype/mock up UIs and while doing that I write quick and usually sub-par code which React can warn about. While I want to fix those warnings, I don't really care until I know that I wont throw away all the code in the next hour at least. Forcing people to fix them instantly will drastically slow down experimental development.

[glenjamin]

For folks who feel the warning box approach may be too intrusive, other libraries (e.g VueJS) already display DOM overlays during your dev/iteration workflow to encourage bug-fixing or slow-paths:

screen shot 2017-01-24 at 10 57 11 am

Are you sure that's from Vue itself? It looks a lot like webpack build errors displayed with the error overlay from webpack-hot-middleware. If this is the case, it's subtly different because it's dev-time build tooling adding the overlay rather than a general-purpose frontend framework.

In general I'm in favour of the warning overlay, but I think it should contain explanatory text on it that says what it is, why it's there, and that it can & should be disabled as part of turning off dev mode. Behind an expando if that's a bit long - but it seems like as good a place as any to get the message across.

[jquense]

I dread updates like 15.2.0 with an overlay. minor bump and suddenly you have literally 100s of warnings about props being passed to DOM nodes. errors maybe, but I don't think depreciation warnings belong in such an intrusive space

[bvaughn]

errors maybe, but I don't think depreciation warnings belong in such an intrusive space

I don't know if this was very clearly communicated prior, but the idea regarding yellow-box warnings (#7360) was not to show all warnings (deprecation or other). Rather, certain warnings that the team deemed to be particularly important would be highlighted this way. The rest would presumably remain in the console.

At least that's my take-away from the conversation Tom and I had about this feature a week or two ago.

[sebmarkbage]

The prop warning in 15.2 was also a mistake IMO and not indicative of our normal M.O. We'd like to have a way to control warning levels by minor versions to avoid that.

[markboyall]

The main reason our team does not use the production build is because we can't run JS unit tests, since the test utils are not included.

[addyosmani]

First, another round of thanks to the React team (@sebmarkbage, @gaearon, @tomocchino and others) for discussing this issue with us and being so open to talking to us about performance & mobile at BlinkOn and other syncs this quarter.

Status check

Per @aweary in #7360, the Yellow Box solution to this particular problem has been put on hold until React’s high-prio V16 work gets completed but should still be happening. facebook/fbjs#165 needs to land and be implemented in Fiber. A good public API for exposing hooks also needs to be crafted. Will be keeping my fingers 🤞

This problem appears to still be prevalent

Quite a few of the production apps that have come across my desk are still shipping DEV mode to production. We can see the When deploying React apps to production debug string in their builds here:

https://cdnjs.cloudflare.com/ajax/libs/react/15.3.1/react.js (tombraider.com)
https://shared.reliclink.com/dlls/vendor-f3e016f6037eb107ffc0.live-shared.min.js (dawnofwar.com)
https://d1xx3pvec9nqb7.cloudfront.net/media/js/thread.af65c1a02d15.js (thread.com)
http://www.sothebys.com/etc/designs/redesigns/sothebys/redesignlibs.min.js (sothebys.com)

I'm still of the view that a pre-Yellow Box move to logging the DEV mode warning to console for the above might have some impact. Sebastian's suggestion of throwing a console error so crash reporting might pick these up was also something I felt was worth consideration.

What else can we do to move the needle here?

Better advocacy? I'm happy to commit to continuing to advocate for folks not shipping DEV mode to production, but do want to see if we can land the official solution post V16 :)

In the short-term, it looks like create-react-appis in a good place to help new projects avoid this problem.

Improvements to installation docs

For everyone else, would there be support for https://facebook.github.io/react/docs/installation.html including a clear, visible callout under the Installing React heading reminding folks to be mindful of DEV mode in production?

As a user, I don't feel there's great incentive for me to read https://facebook.github.io/react/docs/installation.html#development-and-production-versions on first glance otherwise.

Thoughts?

[gaearon]

The main reason our team does not use the production build is because we can't run JS unit tests, since the test utils are not included.

I’m confused about this. Are you running tests on production website? If not, what prevents you from using production build on the production website, and development build in development?

[gaearon]

For everyone else, would there be support for https://facebook.github.io/react/docs/installation.html including a clear, visible callout under the Installing React heading reminding folks to be mindful of DEV mode in production?

Sure. Want to send a PR?

[addyosmani]

Sure. Want to send a PR?

More than happy to.

[jide]

Maybe a word about benchmarks would be nice too to help educating those who compare perfs with react in dev mode ?

[bvaughn]

Makes me think the react devtools extension could be leveraged to display a notification or something obvious when opening a page using react in dev mode maybe ?

I like this idea! I put together a set of proposed icons for the devtools (see facebook/react-devtools/pull/652).

We need to decide how to detect dev vs prod React in a way that's backwards and future safe, but I've added it to the Monday meeting agenda.

[gaearon]

We’ve done some reasonable steps to address this problem:

• React DevTools (with ~700K users) now displays a distinctive red icon for development builds. This helps people learn about the difference between versions early. It also creates some peer pressure, as developers notice this on sites they visit, and report to the people working on them. We’ve seen a few major sites fix the issue within days of rolling this out.

• The notice in React DevTools links to our website where we published instructions to create the production build for all major bundlers. We also made it more prominent on the installation page.

• Create React App continued to gain popularity. It teaches this distinction early with separate commands. It also displays a permanent notice about development mode in the terminal.

• React 16 Beta 1 (and further releases) ship with react.development.js and react.production.min.js as filenames to make it clear that non-minified build should not be used in production.

I think in the future we might explore more ways to solve this problem, but for now I feel like we can move ahead without more drastic measures, and see if it helps. Thanks to everyone for the discussion.