jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.22k stars 507 forks source link

no CJS test build for process.env.NODE_ENV === 'test' #958

Closed techieshark closed 3 years ago

techieshark commented 3 years ago

Current Behavior

As documented here: https://github.com/formium/tsdx#development-only-expressions--treeshaking

// This determines which build to use based on the `NODE_ENV` of your end user.
if (process.env.NODE_ENV === 'production') {
  module.exports = require('./mylib.cjs.production.js');
} else {
  module.exports = require('./mylib.cjs.development.js');
}

... and code here: https://github.com/formium/tsdx/blob/462af2d002987f985695b98400e0344b8f2754b7/src/createBuildConfigs.ts#L54

we see that tsdx emits cjs bundles only for production or development environments, not "test".

Expected behavior

Because another common environment is "test" (at least for jest), I would expect that process.env.NODE_ENV is usable to differentiate "development" and "test" environments.

Suggested solution(s)

I would hope that tsdx would emit a third module, for test mode.

Alternatively, the "non-production" bundle could just not have the process.env.NODE_ENV variable replaced with "development". Here's some code - compiled into the development CommonJS bundle - to tell if my code is running in "development" mode, and you can imagine how it fails if process.env.NODE_ENV is actually "test".

/** Return `true` if `NODE_ENV` indicates we're running in development, `false` otherwise. */
function isInDevelopment() {
  return !!(process.env && "development" === 'development');
 // ------------------------^^^^^^^^^^^^^^^ expect this to remain process.env.NODE_ENV outside of production bundle
}

Additional context

In case it matters, the use case is an internal 'logging' module (based on Winston) which emits colored console logs while running in development, but during Jest tests I want the logs to go to a file instead.

Your environment


  System:
    OS: macOS 10.x

  Binaries:
    Node: 10.22.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.6 - /usr/local/bin/npm

  Browsers:
    Chrome: 87.0.4280.141
    Firefox: 84.0.1

  npmPackages:
    tsdx: ^0.14.1 => 0.14.1 
    typescript: ^3.8.2 => 3.8.2 
agilgur5 commented 3 years ago

Yes, this is actually a duplicate of my very first issue, #167. It was never responded to (prior to me being the maintainer), has received no up-votes in 1.5 years, and I went with a different solution in my library so it has never been prioritized as such.

Unfortunately, implementing a test CJS build is non-trivial since it would have to be optional so that a third CJS build does not increase build times for the vast majority of users that do not need it. You can't currently make dev or prod builds optional, so implementing this effectively requires also implementing #508. We'd also want it to be available in other formats (UMD, SystemJS, etc) for uniformity.

Modifying the dev build to not do any replacement is simpler, but would be a breaking change (in particular, dev CJS would no longer be importable outside of Node due to the env checks being left in) and would decrease the performance of the dev build itself. It would also break uniformity with other build formats which is not ideal

I don't see either being implemented in the near-term, but you should be able to workaround this with some simple patch-package use and potentially with tsdx.config.js (that would be a bit harder since the dev/prod splits are already created by that step).

Closing as duplicate

techieshark commented 3 years ago

Thanks @agilgur5 for the quick response.

FWIW, I switched my build and watch scripts to use tsc and tsc --watch and this is fine for me for now.

ling1726 commented 2 years ago

FYI you can just use process.env["NODE_ENV"] === "test" since the replace plugin used in just matches for process.env.NODE_ENV