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

global URL constructor is mangled to match URL variable in dependency module #10

Closed
ahdinosaur opened this issue Apr 27, 2018 · 4 comments
Labels

Comments

@ahdinosaur
Copy link
Member

hi, thanks for tinyify, it's real rad! 😊

i noticed a bug with the mangled variable names affecting the global URL constructor, i'm not sure exactly which module is responsible so i'm posting here.

in one of my dependency modules (redux-bundler/src/create-url-bundler.js) the code uses the global URL constructor to parse urls.

in another dependency module (ajv/lib/compile/formats.js) there is a local variable called URL (a regular expression to match urls).

for some reason, they are both being mangled to the same minified name, which means the call to new URL() fails because it's now something like new ie() which is not a constructor function.

i made a reproducible test case here: ahdinosaur/tinyify-global-url-bug, which is simply:

var Ajv = require('ajv')

new URL('test')

which with browserify -p tinyify . becomes

... new ie("test")}();

i haven't yet figured out to replicate the same behavior without ajv but hey.

thanks for listening. 🐱

ahdinosaur added a commit to peachcloud/redux-bundler that referenced this issue Apr 27, 2018
ahdinosaur added a commit to peachcloud/redux-bundler that referenced this issue Apr 27, 2018
@goto-bus-stop
Copy link
Member

goto-bus-stop commented Apr 27, 2018

It looks like a problem in browser-pack-flat: (it's usually the one responsible for variable name conflicts so far 😅 )

// ajv.js
var URL = 'abc'

// index.js
require('./ajv')
new URL('test')

With browserify -p browser-pack-flat/plugin index.js becomes

(function(){
var _$ajv_1 = {};
var URL = 'abc'

var _$tinyifyGlobalUrlBug_2 = {};
/* removed: var _$ajv_1 = require('./ajv') */;

new URL('test')

}());

browser-pack-flat is supposed to rename module-global variables when they collide with variable names in other modules, but the URL …global-global… variable is probably not being taken into consideration. Then browser-pack-flat figures that the URL variable in ajv.js is uniquely named and doesn't need renaming, and the new URL call in the final output becomes a reference to the URL variable from the ajv module.

goto-bus-stop added a commit to goto-bus-stop/browser-pack-flat that referenced this issue Apr 27, 2018
@goto-bus-stop
Copy link
Member

Adding a failing test to browser-pack-flat for this case: goto-bus-stop/browser-pack-flat#27
appreciate the detail in this issue and the reproduction!

Unfortunately the fix seems kinda tricky; may need to split up some of the logic in separate steps so that variable names are collected over the entire bundle and then marked as duplicates over the entire bundle, instead of doing the collection and duplicate marking in one step.

goto-bus-stop added a commit to goto-bus-stop/browser-pack-flat that referenced this issue Apr 28, 2018
@goto-bus-stop
Copy link
Member

📦 browser-pack-flat@3.0.9

@ahdinosaur
Copy link
Member Author

thanks @goto-bus-stop 💝 🐈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants