create-react-app icon indicating copy to clipboard operation
create-react-app copied to clipboard

Conditionally drop console messages from production build

Open candrews opened this issue 4 years ago • 14 comments

Add a environment variable, DROP_CONSOLE, to specify if console function calls should be dropped in the production build.

Supersedes https://github.com/facebook/create-react-app/pull/9221/

candrews avatar Jun 25 '20 21:06 candrews

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

stale[bot] avatar Jul 25 '20 23:07 stale[bot]

@eddiemonge @ianschmitz what are your thoughts on this?

I believe it addresses the concerns expressed in my earlier PR, and I think it would be a great improvement to CRA.

Thanks in advance!

candrews avatar Jul 25 '20 23:07 candrews

This seems reasonable to me.

Another way I know of doing this on a per project basis is to create a logging utility function that wraps console call somewhat like this:

export const log = (...args) {
  if (process.env.NODE_ENV !== 'production') {
    console.log(...args);
  }
}

If the env is production then the code is removed at build time.

eddiemonge avatar Jul 27 '20 16:07 eddiemonge

@iansu @amyrlam @mrmckeb @ianschmitz @petetnt Can you please take a look at this PR?

I believe that this approach to removing console statements from production builds is easy, safe, maintainable, and very much desirable for end users.

If there are further changes I can make to this PR to make it more acceptable and/or appealing, please let me know. I'm eager to get (or a functionally equivalent) change accepted.

Thanks again!

candrews avatar Aug 04 '20 16:08 candrews

This PR would be welcomed by the community here https://stackoverflow.com/questions/47839311/removing-log-statements-in-create-react-app-without-ejecting/53770988#53770988

hutch120 avatar Sep 23 '20 03:09 hutch120

Anything I can help with to progress this PR? My two cents... the code change is identical code style to the other environment checks such as shouldUseSourceMap.

I took a look at the two failing checks, but there is no detail on why they failed.

hutch120 avatar Oct 13 '20 22:10 hutch120

Just another note of support for this PR ... using the popular snippet if (!dev) { console.log = () => {} } in a library module will unintentionally remove console.log messages in development mode for modules that use that library.

hutch120 avatar Nov 06 '20 02:11 hutch120

@ianschmitz @eddiemonge @mrmckeb @petetnt (and anyone else) can you please look at this PR? I believe it meets all requirements for a contribution and addresses an issue requested by CRA users. If there's anything I can do to move this along, please tell me - it's been a long time and I'd really like to see some progress (or at least feedback) on this PR.

Thank you in advance!

candrews avatar Nov 24 '20 14:11 candrews

@ianschmitz Thanks for adding the tag new feature, just wondering does that indicate there an ETA for accepting/rejecting the PR? Next major release or something?

hutch120 avatar Feb 01 '21 21:02 hutch120

Any ETA on this? Would be nice to have this implemented guys

yepes avatar May 07 '21 08:05 yepes

Bump

alvaroschipper avatar Aug 25 '21 14:08 alvaroschipper

Can someone please review this

benkingcode avatar Jan 12 '22 17:01 benkingcode

Another gentle nudge. Can someone from facebook (@ianschmitz ?) take a bit of time to approve / give feedback on this? It would be highly appreciated. Eject is not the way.

cliffoo avatar Jul 29 '22 00:07 cliffoo

Just a quick update on this, I still get upvotes on my response to this issue on StackOverflow and the issue has over 10K views. Seems like this is still a relevant topic for some people, personally I've given up on this ever being actioned, but would be nice to have some closure here one way or the other so we can all move on. https://stackoverflow.com/questions/47839311/removing-log-statements-in-create-react-app-without-ejecting/53770988#53770988

hutch120 avatar Sep 27 '22 03:09 hutch120

@jackblackevo @matrush @rvdende @danielrentz @sdarnadeem @pawelskowronek @nvh95 @JSLGeeganage and anyone else - can you please review this (simple) MR and let us know what needs to be done to merge it, or why it cannot be mergeable?

candrews avatar Feb 28 '23 20:02 candrews