adambutler / vuejs-rails

A simple asset-pipeline wrapper for Vue.js by Evan You
MIT License
429 stars 56 forks source link

Provide a way to switch between minified and full versions of assets #39

Closed YurySolovyov closed 7 years ago

YurySolovyov commented 7 years ago

Here is an attempt to make loading assets dynamic depending on current environment. I don't know much about asset pipeline, but I wanted to get things going. Any feedback is appreciated as well as trying out this branch.

This should fix #37

YurySolovyov commented 7 years ago

I also created a new branch with a different approach

ckimdev commented 7 years ago

I like this 👍

YurySolovyov commented 7 years ago

Updated to not rely on current env, but to use Vue.minify flag instead, every application author will need to make a separate decision if minification should depend on env and/or anything else.

The thing I noticed, is that you may not see changes after changing the state of the flag until you clear asset cache if you use it. If that's ok, I can a note to docs.

YurySolovyov commented 7 years ago

@adambutler ping

joxxoxo commented 7 years ago

@YurySolovyov relying on current env seem reasonable default (convention over configuration, etc). Why have you decided to get rid of it?

YurySolovyov commented 7 years ago

I'm actually fine either way, but I though that this way there will be less magic, though given you can still override the flag, I can bring it back

YurySolovyov commented 7 years ago

@joxxoxo Thanks for your ideas, implemented the suggestions.

adambutler commented 7 years ago

Forgive me if I've missed something but come to think about it should the Rails asset pipeline not be handling minification?

YurySolovyov commented 7 years ago

True, the thing is that it is not only about minification. Full builds enable warnings and other debug stuff, while minified versions are not just compressed version of full build, but also have debug stuff ripped off entirely. More info here

adambutler commented 7 years ago

🤦‍♂️ Ah, of coarse that makes sense, as you were.

YurySolovyov commented 7 years ago

The only thing I'd add would be that if you're using cache for asset pipeline, you need to clear cache after switching the state of the flag.

YurySolovyov commented 7 years ago

This should be good to go.

adambutler commented 7 years ago

Setting Vue.minify = true doesn't seem to work for me, even after running rake assets:clean.

I've uploaded my test repo: https://github.com/adambutler/vuejs-rails-test

I'm also not sure minify should be the attribute as its behaviour extends just minification. Perhaps change this to:

Vue.development_mode = false # deafault: true
joxxoxo commented 7 years ago

@adambutler I guess you have //= require vue in applicaion.js locally, right?

You may need to do rm -rf tmp/cache/assets to ensure cache is clean.

I've pulled your repo, added require to application.js, added route for static#landing and it works for both minified and not minified version

YurySolovyov commented 7 years ago

@adambutler I've checked out your repo and made changes suggested by @joxxoxo. You're right that rake assets:clean didn't not work, so I updated the suggested task to run and it works Please try again. Also renamed the flag as you pointed out.

YurySolovyov commented 7 years ago

@adambutler anything I can do to move this forward?

ckimdev commented 7 years ago

excited for this PR to get merged 🥇

ckimdev commented 7 years ago

is this getting merged anytime soon?

schmidtg commented 7 years ago

I like the approach. LGTM!

YurySolovyov commented 7 years ago

Thanks a lot, there are newer Vue and Vue Router out there, we might want to do a new release and also include these changes