Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an easy way to remove minification #310

Open
tychota opened this issue Nov 7, 2019 · 8 comments
Open

Provide an easy way to remove minification #310

tychota opened this issue Nov 7, 2019 · 8 comments
Labels
kind: feature New feature or request

Comments

@tychota
Copy link

tychota commented Nov 7, 2019

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:

@swyxio
Copy link
Collaborator

swyxio commented Nov 7, 2019

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
Copy link
Contributor

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

@techieshark
Copy link
Contributor

@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
Copy link
Collaborator

agilgur5 commented Oct 21, 2020

@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
Copy link
Contributor

@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
Copy link
Collaborator

@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
Copy link

adriano-di-giovanni commented Mar 11, 2021

@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
Copy link

faloi commented Jan 7, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants