search

PlasmoHQ / plasmo

🧩 The Browser Extension Framework
https://www.plasmo.com
MIT License
8.84k stars 314 forks source link

chore: drop console.log from production builds #1012

Closed flexchar closed 3 days ago

flexchar commented 2 weeks ago

Details

This PR enables drop_console option in swc minifier.

I tried to add support for custom .swcrc but I couldn't get it work in any way. It would be amazing if people could configure that. But I also think it's ok to not have console log in production. What do you think?

Options docs: https://swc.rs/docs/usage/core#options Compress docs: https://swc.rs/docs/configuration/minification#jscminifycompress

Related: https://github.com/PlasmoHQ/plasmo/issues/542

Code of Conduct

Contacts

If your PR is accepted, we will award you with the Contributor role on Discord server.

To join the server, visit: https://www.plasmo.com/s/d

mathieudutour commented 5 days ago

I don't particularly agree with this change: there are valid reasons to want logs in prod build. The way to usually deal with it is to have a logging utility where you choose which level of logs you want to keep in prod build based on a variable in process.env (there are tons of utilities like that, https://github.com/debug-js/debug is one of them)

flexchar commented 4 days ago

@mathieudutour that's a good point, that's why I added an open question to my PR. Ideally i would like to support .swcrc file to let people customize but after hours I just couldn't get it to work. I think it is because it is not supported when contents are passed as a string. Do you agree @louisgv?

It could also be a CLI option but it feels wrong. It feels that something like this should live in a dedicated config file.

louisgv commented 3 days ago

Hmm @flexchar yeah I think most sane solution would be to suggest end-user to leverage env vars. Parcel prunes dead branches. Thus:

if (process.env.NODE_ENV === 'development') {
     console.log("etc")
}

Will be pruned if it's production

flexchar commented 3 days ago

Oh, I didn't know this existed. I did have simple function log() that had this in the body but Parcel wasn't smart enough to remove those. This is because SWC processes each file individually so it is not aware of how imported functions behave. This is something I am generally really not fond of the way it work. It also prevents some other features. Hence the inspiration for this.

I am definitely less experienced in the context of custom parcel + swc. Do you think there is a relatively easy way to provide config file?