martyjs / marty

A Javascript library for state management in React applications
http://martyjs.org
MIT License
1.09k stars 76 forks source link

ES6 classes lose their 'id' property when minified. #291

Closed jimmed closed 9 years ago

jimmed commented 9 years ago

When using Marty.register with ES6 classes, the optional id property must be passed. If it is not, minifying the code will create naming collisions between classes when registered with Marty's registry.

I'm not sure whether it is best to investigate an alternative strategy for inferring the id of the class being registered, or simply to add a warning in the documentation.

jhollingworth commented 9 years ago

Ah thats annoying, I should have thought of that. Can't get to a computer to test this but i'm wondering if passing --keep-fnames to uglify (assuming you're using uglify) would fix it? Not ideal but perhaps better than getting everyone to add id as a property.

class UserStore extends Marty.Store {
   get id { returns "UserStore" }
}

This problem should go away with the new way of registering everything in v0.10

dariocravero commented 9 years ago

Hey @jimmed, we're solving this issue at uglify level with the following:

// package.json
{
"scripts": {
    "build-app": "NODE_ENV=production browserify app.js > dist/app.js && uglifyjs dist/app.js -cm -r `.bin/extract-classes` --screw-ie8 > dist/app.min.js"
}
# .bin/extract-classes
# !/usr/bin/env bash
find . -name '*.js' -not -path './dist/*' -not -path './public/*' -not -path './node_modules/*' -exec awk '/export default class / {print $4};/export class / {print $3};' {} \; | tr "\\n" ","
idolizesc commented 9 years ago

Any workaround for this that doesn't involve running bash scripts? I'm currently using uglifyify with Browserify inside my Gulp script, and I would love to just pass some setting to keep things working while I wait for v0.10.

jhollingworth commented 9 years ago

@idolizesc sorry for the delay. Untested but I think this might work

 return gulp.src('lib/*.js')
    .pipe(uglify({ keep_fnames: true }))
    .pipe(gulp.dest('dist'));
jhollingworth commented 9 years ago

This has been solved in v0.10