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

Uglify terser copy #256

Closed
wants to merge 2 commits into from
Closed

Uglify terser copy #256

wants to merge 2 commits into from

Conversation

haggholm
Copy link
Contributor

Summary

Add support for using terser. terser is a fork of uglify-es, which is not really maintained; and maintains API compatibility with uglify-es.

In order to avoid duplicating configuration like default options in metro-minify-uglify, this PR adds a parameter to metro-minify-uglify that allows passing in an alternative minifier module, and the new package metro-minify-terser calls this, passing in terser.

This is based on discussion in #230.

After earlier submitting #243, I created this alternative PR to address comments from @rafeca

Test plan

The tests are simply copied from metro-minify-uglify, since the API is and the results should be identical.

…l instances of `uglify(-es)?` replaced with `terser`.
…erser` a (modified) copy of `-uglify`; (2) remove the `noSourceMap` function (and associated tests &c.); (3) move Flow types from minifier to core `metro` package.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 17, 2018
@haggholm haggholm mentioned this pull request Sep 17, 2018
Copy link
Contributor

@rafeca rafeca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Thanks a lot!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rafeca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@DimitryDushkin
Copy link

Hey! How to use it?

@skurgansky-sugarcrm
Copy link

I would like to know too..

@DimitryDushkin
Copy link

I would like to know too..

Hey! For RN 57 this rn-cli.config.js works:

module.exports = {
    transformer: {
        minifierPath: 'metro-minify-terser'
    }
};

also
npm i -D metro-minify-terser

@skurgansky-sugarcrm
Copy link

@DimitryDushkin thanks for response ! btw in RN 0.59.1 there is new metro.config.js. should I use it to replace rn-cli.config.js ? It's a pity there was no comments in docs about that.

@radiosilence
Copy link

radiosilence commented Feb 3, 2022

Hey @haggholm I'm trying to use this now and I get this error:

The reason I am trying it is due to this issue: #645

And using inbuilt terser support instead of using a custom config seemed like a better idea

> Task :app:bundleReleaseJsAndAssets
warning: the transform cache was reset.
                    Welcome to Metro!
              Fast - Scalable - Integrated


TypeError: Cannot read properties of undefined (reading 'match')
    at countLines (/Users/jc/Workspace/sbf/sbf.app.inspection/node_modules/metro/src/lib/countLines.js:17:23)
    at transformJS (/Users/jc/Workspace/sbf/sbf.app.inspection/node_modules/metro-transform-worker/src/index.js:332:20)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async transformJSWithBabel (/Users/jc/Workspace/sbf/sbf.app.inspection/node_modules/metro-transform-worker/src/index.js:408:10)
    at async Object.transform (/Users/jc/Workspace/sbf/sbf.app.inspection/node_modules/metro-transform-worker/src/index.js:569:12)
info Run CLI with --verbose flag for more details.
error index.js: Cannot read properties of undefined (reading 'match').

My metro config:


module.exports = {
  transformer: {
    getTransformOptions: async () => ({
      transform: {
        experimentalImportSupport: false,
        inlineRequires: true,
      },
    }),
    minifierPath: "metro-minify-terser",
  },
};

@geraintwhite
Copy link

@radiosilence did you figure out a workaround for this?

@robhogan
Copy link
Contributor

robhogan commented Apr 7, 2022

If you're seeing Cannot read properties of undefined (reading 'match')., be sure to use a version of metro-minify-terser that matches your metro version. See #767.

@geraintwhite
Copy link

Thanks, I suppose installing a newer version of metro doesn't work as it's tied to the react-native version (my package.json has metro, metro-minify-terser and metro-react-native-babel-preset at 0.70.1).

@robhogan
Copy link
Contributor

robhogan commented Apr 7, 2022

Yes - manually adding metro to your project dependencies probably won't behave as you want, and isn't recommended - when you use the CLI you'll be using the version deep in your dependencies:

react-native -> @react-native-community/cli -> @react-native-community/cli-plugin-metro -> metro

If you're using Yarn you could (via resolutions) force @react-native-community/cli-plugin-metro onto a different version of metro, and that should work, but I haven't tried it and it's in no sense supported.

React Native 0.68 will give you Metro 0.67 which includes the latest minifier-related updates, so updating to that is another option. Otherwise, you'll need to downgrade metro-minify-terser to 0.66.

(This confusion should be sorted out soon-ish - I'm waiting for terser to cut a new release after terser/terser#1164, then we'll make metro-minify-terser the pre-bundled Metro default.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants