Provide an easy way to remove minification

tychota commented 5 years ago

Current Behavior

Currently, TSDX (v0.9, couldn't upgrade to 0.10 due to various problems already reported) behavior concerning minification is:

if (opts.minify !== null && opts.minify !== null) { 
 // respect the set option
} else {
 switch (process.env.NODE_ENV) {
  case "PRODUCTION":
    opts.minify = true;
    break;
  defaut:
    opts.minify = false
  }
}

// later
if (opts.minify) {
  plugins.add(new Terser({});
}

The impact on me is the following. When building a NestJS, the swagger looks like this.

💩

There is an issue for Terser mangle options: #136

There is also a possibility to use the new "tsdx.config.js" to remove the plugin

// Not transpiled with TypeScript or Babel, so use plain Es6/Node.js!
module.exports = {
  // This function will run for each entry/format/env combination
  rollup(config, options) {
    config.plugins = config.plugins.filter(plugin => plugin.name !== 'terser');
    return config; // always return a config.
  },
};

But that is not easy to do and undocumented

Desired Behavior

An easy and documented way to disable minification.

Who does this impact? Who is this for?

Me :) And my team
all user wanting to remove minification, such as nestjs user

Describe alternatives you've considered

As stated in Current Behavior:

136
tsdx.config.js

swyxio commented 5 years ago

at a minimum i’m happy to take a documentation pr with those exact keywords. im always wary of adding a new flag because of flag sprawl, so that can be a separate discussion.

ambroseus commented 4 years ago

@tychota current version already have --minify option https://github.com/jaredpalmer/tsdx#rollup

techieshark commented 4 years ago

@ambroseus I'm not seeing any "minify" related CLI arguments for the tsdx build command:

https://github.com/formium/tsdx/blob/569c3ed36863d0cbe3f869f397d1852beefd8d5d/src/index.ts#L363

I see the following in the Rollup config section you link to, but if that's supposed to be a way to turn off minification, it's not clear (to me anyway, from those docs) how to do so.

// Is minifying?
  minify?: boolean;

If anyone else knows how to disable minification or can point to something that documents it that'd be appreciated. Thanks!

agilgur5 commented 4 years ago

@techieshark could you elaborate on your use case for disabling minification? A workaround was provided by OP using tsdx.config.js and #136 is possible via that route as well. If others still want this for other use-cases, I'd like to know what those are to shape decisions.

I'm not seeing any "minify" related CLI arguments for the tsdx build command:

There isn't one, though tsdx build --minify=0 unintentionally works due to this line of code: https://github.com/formium/tsdx/blob/6c5da7444b7db006c9e3ec932006528b367bcd59/src/createRollupConfig.ts#L37 Notably, I'm using 0 instead of false because it's not an official option and so it isn't parsed; it only works because Boolean(0) === false

That's quite hacky and relies on that line not changing and the fact that options get passed all the way through the CLI via sade even if they're not explicitly defined, so I'm not sure I'd recommend that (but if an official flag is created, it may use that same name), but it does work.

if that's supposed to be a way to turn off minification, it's not clear (to me anyway, from those docs) how to do so.

It isn't, that's something you can use inside of your tsdx.config.js to tell if the current bundle being generated will be minified and write your own conditional around that. For instance, modifying OP's workaround:

tsdx.config.js:

module.exports = {
  rollup(config, options) {
    if (options.minify) {
      // disable Terser's minification
      config.plugins = config.plugins.filter(plugin => plugin.name !== 'terser');
      // remove `.min` suffix since we're no longer minifying
      config.output.file = config.output.file.replace('.min', '');
    }
    return config;
  },
};

I've modified output.file here as well, but it would be prudent to note that output.file will be substituted with output.dir and output.entryFileNames in v0.15.0 with #367, so it's not quite stable either.

techieshark commented 4 years ago

@agilgur5 thanks for the explanation! And the clever hack. Very helpful. :)

I was actually using tsdx to build a little Node/TS service for cloud deployment, and wanted to turn off minification because doing so would have made cloud-based debugging a bit easier/quicker. I later realized there was also the unminified development file, so that helped too.

agilgur5 commented 4 years ago

@techieshark the .esm file is also unminified as that relies on bundlers to minify, but if you're building a Node library then your consumers wouldn't be minifying anyway, so you could use that too if you're only supporting Node 12+

adriano-di-giovanni commented 3 years ago

@agilgur5 can't remove the .min suffix because the output index.js requires it


'use strict'

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./my-library.cjs.production.min.js')
} else {
  module.exports = require('./my-library.cjs.development.js')
}

Correct?

faloi commented 1 year ago

Run into the exact same problem with NestJS and Swagger, but the real issue is that the minify process renames the class to "m" or "u" or any other letter:

  (exports.CreateDepartamentoDto = u),
  (exports.CreateFueroDto = m),
  (exports.UpdateDepartamentoDto = g),
  (exports.UpdateFueroDto = l);

The issue is with the compressing and mangling of the function names, which I managed to disable like this:

   // Remove terser plugin
   config.plugins = config.plugins.filter(plugin => plugin.name !== 'terser');

   // Add it again, with `keep_fnames` option set to true and the default options found on
   // https://github.com/jaredpalmer/tsdx/blob/master/src/createRollupConfig.ts#L214.
   config.plugins.push(
      terser({
        output: { comments: false },
        compress: {
          keep_infinity: true,
          pure_getters: true,
          keep_fnames: true,
          passes: 10,
        },
        ecma: opts.legacy ? 5 : 2020,
        module: isEsm,
        toplevel: opts.format === 'cjs' || isEsm,
        warnings: true,
        mangle: {
          keep_fnames: true,
        },
      }),
    );

This could be improved by getting the original plugin and overwriting the desired options, but it seems like it cannot be done because it only exposes a name and renderChunk properties:

{ name: 'terser', renderChunk: [Function: renderChunk] }

jaredpalmer / tsdx