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

fix root v-else not rendering in prod and switched examples to minified vue for better prod coverage #3943

Merged
merged 3 commits into from
Oct 14, 2016
Merged

fix root v-else not rendering in prod and switched examples to minified vue for better prod coverage #3943

merged 3 commits into from
Oct 14, 2016

Conversation

chrisvfritz
Copy link
Contributor

@chrisvfritz chrisvfritz commented Oct 14, 2016

Fixes #3940. This was a bit of a brainless error on my part - I apologize. Something I noticed however, is that none of our tests are currently covering the production build of Vue, thus allowing this error to slip through. To prevent this in the future, I was wondering if we should split the unit tests into two groups:

  • Development-specific behavior (e.g. warnings)
  • Features that should behave the same in both dev and prod, which will perform a second run with NODE_ENV set to production

I think that work would be well outside the scope of this PR, but as a stop-gap measure to cover production at least a little, I switched all of the e2e examples to use the production build, then tweaked the grid example so that I could add a regression test.

My thinking is that production is probably most important for e2e anyway, as devs will more quickly discover development-only errors (and they won't affect end users). The only downside I can foresee is that if someone clones the repo to start playing with examples, they won't get any nice warnings if they make a mistake. So, I added a comment above each Vue script with instructions to use the development version if they'd like. Still, I'm happy to change it back if this is undesirable. We'd just have to think of another way to add a regression test.

One last unrelated note: while updating the examples, I noticed that the todomvc's HTML file was full of tabs rather than spaces, so I converted them for consistency.

@yyx990803 yyx990803 merged commit 4f5a47d into vuejs:dev Oct 14, 2016
@chrisvfritz chrisvfritz deleted the bugfix/root-v-if-else-in-production branch October 14, 2016 17:19
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