[Bug] Removal of quotes for property names with special unicode still broken

[patrick-steele-idem]

Despite PR #543 - Fix removal of quotes from property names, there is still a problem with how quotes are removed from property names that can be noticed when trying to load the minified he library in the latest Safari.

To clarify, given the following input file:

module.exports = {
    '\uD835\uDCB6': 'ascr'
};

The babili output is the following:

module.exports={𝒶:'ascr'}

If you try to load that in Safari you get the following error:

SyntaxError: Invalid character '\u55349'

Thoughts?

[patrick-steele-idem]

/cc @boopathi @mathiasbynens

[patrick-steele-idem]

Also, please see my earlier comment:

I did some additional investigation and found that the babel-plugin-minify-constant-folding plugin will also introduce Identifier nodes for object keys even if the key name contains special unicode characters because t.isValidateIdentifier() doesn't restrict to safe identifiers (safe for currently the latest Safari v10.0.3, that is). The babel-plugin-transform-property-literals plugin only transforms string literal keys (not identifier keys). Therefore, even with this patch, he does not get minified in a way that can be loaded in the latest Safari.

It looks like both babel-plugin-minify-constant-folding and babel-plugin-transform-property-literals are causing problems.

[timsuchanek]

This is indeed a problem, our build is now working in Firefox with the changes of #543 but still doesn't work with Safari. So this is the last issue holding us back from migrating completely to babili

[damianobarbati]

Using babili I get the following!

SyntaxError: Invalid character '\u2160'

Around here:

Copyright (c) 2016 Jed Watson.
  Licensed under the MIT License (MIT), see
  http://jedwatson.github.io/classnames
*/ <== console brings me here!

[mathiasbynens]

@damianobarbati Can you link to a test case that reproduces the issue? That comment doesn’t include U+2160.

[damianobarbati]

Unfortunately I can't 😞
And I don't know why the console brings me over there.
If I manage to reproduce this in a repro I'll be sharing that here for sure, thanks @mathiasbynens

[spudly]

I just ran into the same problem as @patrick-steele-idem.

Worked around the issue by adding the following options to the minify command: --no-evaluate --no-propertyLiterals

This disables the two plugins that are breaking the output code (babel-plugin-minify-constant-folding and babel-plugin-transform-property-literals).

[JayCarney]

For anybody running into this using babel-minify-webpack-plugin worked around this using the advice from @spudly using:

const MinifyPlugin = require('babel-minify-webpack-plugin');

// webpack settings
plugins: [
  new MinifyPlugin({
    evaluate: false, 
    propertyLiterals: false
  })
]

[Awk34]

I'm getting the same issue with https://github.com/mozilla/pdf.js . The first place it starts failing is for \u2160, which is Roman Numeral One (Ⅰ). The same workaround to disable property minification fixed it for me. Here's the line in their source code: https://github.com/mozilla/pdf.js/blob/v1.9.426/src/core/unicode.js#L335

[jogibear9988]

seems like the conversation of the literal is already done in babel.

[jogibear9988]

In "babel-plugin-minify-constant-folding" it can be solved by inserting

	if (t.isObjectExpression(node)) {
	  for (var p of node.properties) {
		 if ((p.key.extra.raw[0] == '\'' && p.key.extra.raw != '\'' + p.key.extra.rawValue + '\'') ||
             (p.key.extra.raw[0] == '\"' && p.key.extra.raw != '\"' + p.key.extra.rawValue + '\"'))
			return;
	  } 
    }

on line https://github.com/babel/minify/blob/master/packages/babel-plugin-minify-constant-folding/src/index.js#L144

[jogibear9988]

In polymer cli i also get this code:

 var identifier = "(?:\\\\.|[\\w-]|[^\0-\\xa0])+";

from jquery to break: https://github.com/Polymer/polymer-cli/issues/886