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

Run tests in production and development modes #29

Merged
merged 2 commits into from
Oct 18, 2017
Merged

Conversation

tomdale
Copy link
Contributor

@tomdale tomdale commented Oct 18, 2017

Note: This PR started off as an investigation into why a test I wrote against tracked properties and frozen objects wasn't failing, and metastasized from there. 😂

It is very common for JavaScript libraries to have two "modes": one that developers can run locally to get better error messages or other helpful feedback, and another mode where assertions and other non-critical code with runtime overhead is stripped out to provide the best performance for users.

Both the Glimmer VM and Glimmer.js conceptually have a "development" mode and a "production" mode. Perhaps the most notable example of the difference between development mode and production mode in Glimmer.js is what happens with tracked properties. In development mode, we install a getter even on properties that are not marked as tracked, so we can emit a helpful error if a developer inadvertently mutates that property and expects the DOM to update. Because of the non-trivial runtime cost of installing a setter on every property of every object that passes through a template, we strip this functionality in production mode.

To do this stripping, we use a pattern that looks like this:

import { DEBUG } from '@glimmer/env';

if (DEBUG) {
  assert(object.isWorking, 'the object has stopped working');
}

In @glimmer/env, DEBUG is a const boolean, so bundlers like Rollup will transform the above code into something like this:

const DEBUG = false;

if (DEBUG) {
    assert(object.isWorking, 'the object has stopped working');
}

When that goes through a minifier like Uglify, the minifier will notice the conditional contains a constant boolean and the entire conditional branch will be stripped out (in this case, leaving an empty file).

Which brings us to this PR. This PR addresses several issues with the current build/test setup:

  1. At some point, the tests stopped running in development mode. A number of tests we intended to verify development-mode features simply never ran.
  2. The reason these stopped running is because we were relying on the babel-plugin-debug-macros plugin to statically replace imports from @glimmer/env with literal boolean values based on the current environment.
  3. When this got dropped (almost certainly my fault) the bundler started picking up values from the version of @glimmer/env on npm, where DEBUG is always false.
  4. There seems to be some confusion about @glimmer/env and @glimmer/local-debug-flags, which appear to have quite a bit of overlap.
    1. This repository contained a local copy of @glimmer/local-debug-flags, which was unused.
    2. In production builds only, we were attempting to statically replace @glimmer/local-debug-flags instead of @glimmer/env. This "bug" was actually beneficial, because without it we actually would not have produced any build artifacts that contained the development mode assertions!
    3. In this PR, I removed @glimmer/local-debug-flags entirely. The Babel plugin is now configured to rewrite imports from @glimmer/env, allowing us to toggle when they are enabled or disabled as intended.
  5. With this PR, we no longer apply any kind of production-mode stripping to the artifacts distributed on npm. This keeps them as close to the original source as possible, and allows application build environments (like @glimmer/application-pipeline) to keep dev-mode assertions during development but strip them in production builds.
    1. The downside is that someone importing and using packages directly from npm may end up with non-optimized bundles without realizing it.
    2. This is mitigated somewhat by the published @glimmer/env having debug flags set to const false values. As described above, building with better bundlers like Rollup and passing through a minifier will produce the optimized build "automatically."
    3. We should consider adding a warning when we detect that the library is running in development mode so people are at least aware if their bundling strategy is not producing optimized code.
  6. While we stop applying debug flag rewriting to the packages themselves, we now do apply rewriting to the final test bundle, getting us very close to testing the actual path Glimmer.js apps use to consume these packages.
    1. In order to verify that both modes are working correctly, this PR changes the test command to run tests in both production and development modes.
  7. To ensure more complete test coverage, this PR also adds some tests for what should not happen in production mode.

@tomdale tomdale merged commit 9e0e157 into master Oct 18, 2017
@tomdale tomdale deleted the dev-mode-tests branch October 18, 2017 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant