tsdx icon indicating copy to clipboard operation
tsdx copied to clipboard

Provide an easy way to remove minification

Open tychota opened this issue 5 years ago • 8 comments

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.

image 💩

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.

Suggested Solution

  • Add --minify false to nest CLI
  • Add documentation in the main README.md

I will be super happy to contribute with a PR if you agree on the expected behavior

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

tychota avatar Nov 07 '19 09:11 tychota

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.

swyxio avatar Nov 07 '19 12:11 swyxio

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

ambroseus avatar Dec 18 '19 17:12 ambroseus

@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!

techieshark avatar Oct 21 '20 21:10 techieshark

@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.

agilgur5 avatar Oct 21 '20 23:10 agilgur5

@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.

techieshark avatar Oct 23 '20 23:10 techieshark

@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+

agilgur5 avatar Oct 25 '20 01:10 agilgur5

@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?

adriano-di-giovanni avatar Mar 11 '21 11:03 adriano-di-giovanni

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] }

faloi avatar Jan 07 '23 22:01 faloi