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

Migrate to UglifyJS v3 and use its comment preservation feature #24

Merged
merged 1 commit into from
Dec 21, 2017

Conversation

natevw
Copy link
Contributor

@natevw natevw commented Oct 17, 2017

At least as of the current Uglify3, it has builtin support for comments preservation. This migrates to that version.

This may have been requested via a mysterious "issue #7" mentioned in a few others, which GH won't let me access for some reason. This should also fix a number of other open issues (probably #19 and #20; not sure about #21).

Note that a few of the default uglify options had to change since they were moved around and/or no longer available. I preserved the ones that I could, and updated the tests to not worry so much about the specific defaults.

…ad of a custom regex one. this should fix a number of open issues (probably yui#19 and yui#20; not sure about yui#21)
@natevw
Copy link
Contributor Author

natevw commented Oct 18, 2017

@davglass Do you think you'll have a chance to review and possibly publish this on npm soon? I'm running into an issue where django-pipeline is intermittently giving us bad JS builds due to the current yuglify comment handling, and need to deploy some resolution to that in the next couple days if possible.

@natevw
Copy link
Contributor Author

natevw commented Oct 25, 2017

With uglify-js@3.1.4, we ended up running into some issues with hoist_vars: true (which I set based on the original lift_vars: true of older versions). The UglifyJS documentation discourages using it, so I wonder if it just hasn't been tested much.

Anyway, this project seems thoroughly dead so instead of updating this patch, I've opted to just reconfigure django-pipeline to use Uglify and CSS Min directly:

PIPELINE = {
# old
#    'YUGLIFY_BINARY': os.path.join(HG_ROOT, 'node_modules/.bin', 'yuglify'),

# new
    'JS_COMPRESSOR': 'pipeline.compressors.uglifyjs.UglifyJSCompressor',
    'UGLIFYJS_BINARY': os.path.join(HG_ROOT, 'node_modules/.bin', 'uglifyjs'),
    'UGLIFYJS_ARGUMENTS': '--mangle --compress --comments /^!/',
    'CSS_COMPRESSOR': 'pipeline.compressors.cssmin.CSSMinCompressor',
    'CSSMIN_BINARY': os.path.join(HG_ROOT, 'node_modules/.bin', 'cssmin'),

    # …other config…
}

Likewise replacing "yuglify": "github:natevw/yuglify#uglify3-comments" in package.json with "cssmin": "^0.4.3", "uglify-js": "^3.1.5".

@davglass davglass merged commit 29a5a16 into yui:master Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants