Skip to content
This repository has been archived by the owner on Oct 2, 2020. It is now read-only.

Different cache folder for different envs #76

Closed
11 tasks done
fasterthanlime opened this issue Jun 21, 2017 · 3 comments
Closed
11 tasks done

Different cache folder for different envs #76

fasterthanlime opened this issue Jun 21, 2017 · 3 comments

Comments

@fasterthanlime
Copy link
Contributor

fasterthanlime commented Jun 21, 2017

This issue applies both to electron-compile and electron-compilers, sorry

Reading through a bunch of code right now to find out if a different $TEMP/compileCache_${hash} will get generated if NODE_ENV changes, and.. it doesn't like it does?

Here's the default cache directory code:

export function calculateDefaultCompileCacheDirectory() {
  let tmpDir = process.env.TEMP || process.env.TMPDIR || '/tmp';
  let hash = require('crypto').createHash('md5').update(process.execPath).digest('hex');

  let cacheDir = path.join(tmpDir, `compileCache_${hash}`);
  mkdirp.sync(cacheDir);

  d(`Using default cache directory: ${cacheDir}`);
  return cacheDir;
}

The problem is:

  • One might want to enable code coverage (or other babel plugins) only in testing
  • But since all files are cached by their content
  • And the files wouldn't change, only the compiler settings would
  • Then electron-compile would just happily serve the old cached versions (which may have coverage instrumentation when they shouldn't, and the other way around).

You can probably reproduce the problem easily if you use electron-compile with a .babelrc like:

{
  "env": {
    "production": {
      "presets": ["es2016-node5", "react"],
      "sourceMaps": false
    },
    "development": {
      "presets": ["es2016-node5", "react"],
      "sourceMaps": "inline"
    }
  }
}

run it once with NODE_ENV=production, then again with NODE_ENV=development, I'm 80% confident you'd end up not seeing sourceMaps still.


Here's my proposal for electron-compile:

  • Change calculateDefaultCompileCacheDirectory to feed off of

Tangentially, here's what I've got planned for electron-compilers:

  • Add way for compilers to get an input sourceMap
    • compilerContext seems okay
  • Make babel compiler consume that input sourceMap when it's there
  • Drop sorcery dependency
  • Make both coverage options (typescript & babel) only apply when NODE_ENV=test
    • Alternatively: only do env matching when coverage is a string, and coverage: true would always instrument, so that the old behavior is retained?
    • Alternatively: don't change anything about the coverage option, force folks to use env blocks
    • see fasterthanlime@a98839a
  • Make both coverage option use babel-plugin-istanbul (a relatively thin wrapper over new-istanbul's instrumenting lib).
    • For babel, this means pushing adding a plug-in to the plugins list
    • For typescript, this.. requires babel :/
    • see fasterthanlime@a98839a
  • Drop dependency to istanbul 0.4.5 (legacy istanbul) - it's somehow still in electron-compile's package.json
  • Document how to set up typescript+babel (babel block in the typescript config)
  • Document how to set up coverage instrumentation
  • Document what coverage instrumentation does (collects coverage info using the new istanbul, aka istanbuljs/istanbuljs, into the global __coverage__ variable)
  • Give an example of collecting/reporting from a __coverage__ variable, with proper sourceMap support (it's like 8 lines).

Let me know if any of that sounds bad @paulcbetts & others so I can adjust plans accordingly!

@fasterthanlime
Copy link
Contributor Author

fasterthanlime commented Jun 21, 2017

Additional notes on [email protected] being deprecated: it only became clear to me after spending a day investigating, but: the acorn-based istanbul is pretty dead.

The new istanbul is heavily marketed as nyc (its command-line interface), it's pretty much everything https://istanbul.js.org/ talks about - but there's actual libs underneath, conveniently stashed in this monorepo.

The new instrumenter is based on babel, which makes a bunch of sense:

  • If you're already using babel, then the cost of istanbul-lib-instrument is next to non-existent
  • If you're not, well.. at least it's more reliable than acorn was.

Quick size comparison:

Legacy istanbul

istanbul-only ツ npm i --save istanbul

+ [email protected]
added 60 packages in 5.097s
 istanbul-only ツ du -ch node_modules | tail -1
9.4M    total

babel-plugin-istanbul

babel-plugin ツ npm i --save babel-plugin-istanbul

+ [email protected]
added 107 packages in 8.772s
 babel-plugin ツ du -ch node_modules | tail -1
18M     total

N.B: size of the instrument+coverage+babel plug-in only:

babel-plugin ツ du -ch node_modules/istanbul-lib-* node_modules/babel-plugin-istanbul | tail -1
164K    total

TL;DR istanbul is now babel-based, it kinda blows if you're not already using babel, but I don't think electron-compile{,rs} should keep using [email protected]

fasterthanlime added a commit to fasterthanlime/electron-compilers that referenced this issue Jun 21, 2017
See electron-userland#76

- Drop [email protected] dependency, add babel-plugin-istanbul
- Add getEnv() helper to SimpleCompilerBase
- Support an environment string in the `coverage` option for both typescript and babel compilers
- Make babel compiler use compilerContext as compile options
- Drop sorcery dependency, pass inputSourceMap to babel instead
- Always use babel+babel-plugin-istanbul to instrument code for measuring coverage
- In ts+babel, translate ts sourcemap options to babel. With a twist, for
inline sourcemaps.
@fasterthanlime
Copy link
Contributor Author

All done in:

Works great for me (typescript+babel, inline source maps):

coverage-demo

@fasterthanlime
Copy link
Contributor Author

Closing in favor of both PRs.

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

No branches or pull requests

1 participant