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.

Sign up for GitHub

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

Jump to bottom

Add caching #77

Closed
wants to merge 6 commits into from
Closed

Add caching #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9e7f4c2
Add caching
Nov 25, 2017
05336fa
Fix dependency config
joeyparrish Aug 27, 2023
c12c3e0
Add .cache to .gitignore
joeyparrish Aug 27, 2023
02fd7b6
Make the cache path configurable
joeyparrish Aug 27, 2023
2fb7d36
Remove .cache from .gitignore, now that the path is configurable
joeyparrish Aug 27, 2023
5aff3bb
Remove fs
joeyparrish Aug 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,31 @@ module.exports = function (config) {
};
```

### Caching

Adding the field `cachePath` will enable caching. Cached output from babel
will be stored into that file, along with md5 hashes of the original contents
of each file. If an original file hasn't changed, the cached babel-processed
results will be reused.

```js
module.exports = function (config) {
config.set({
preprocessors: {
'src/**/*.js': ['babel'],
'test/**/*.js': ['babel']
},
babelPreprocessor: {
cachePath: '.babel-cache',
options: {
presets: ['@babel/preset-env'],
sourceMap: 'inline'
},
}
});
};
```

### Don't preprocess third-party libraries

Third-party libraries may not work properly if you apply `karma-babel-preprocessor` to them. It also introduces unnecessary overhead. Make sure to explicitly specify files that you want to preprocess.
Expand Down
43 changes: 34 additions & 9 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

var babel = require('@babel/core');

var LocalCache = require('node-localcache');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reluctant to add this package as a dependency. It has more dependencies with fixed versions and hasn't been maintained for 7 years.

Can caching be implemented without additional dependencies or a more popular and maintained package?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seconded.

The implementation of node-localcache writes all cache entries as one big JSON object to a single file. This sounds bad for a preprocessor: if you're running Karma in watch mode, a small change to a single file inside a large project means (repeatedly) overwriting a massive JSON file. 😕

I would prefer an alternative caching solution that is more gentle with the file system.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I inherited that from the original PR (#34). Writing files without deps is not hard. And writing to individual files instead of one big one is also easy. I'll revise it.

Do you require old-style callbacks in this project or can I use fs/promises?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Babel 7 supports Node 6 and up, which unfortunately means that both fs.promises and util.promisify() are off the table... 😞 (Or we'd have to wait for Babel 8...)

var crypto = require('crypto');

// @param args {Object} - Config object of custom preprocessor.
// @param config {Object} - Config object of babelPreprocessor.
// @param logger {Object} - Karma's logger.
Expand All @@ -10,20 +13,38 @@ function createPreprocessor(args, config, logger, helper) {
var log = logger.create('preprocessor.babel');
config = config || {};

function preprocess(content, file, done) {
log.debug('Processing "%s".', file.originalPath);
var cachePath = config.cachePath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this work with custom processors as well? var cachePath = args.cachePath || config.cachePath;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I'll do that. I wasn't familiar with that detail.

var cache = null;
if (cachePath) {
cache = new LocalCache(cachePath);
}

function preprocess(content, file, done) {
try {
var options = createOptions(args, config, helper, file);
file.path = options.filename || file.path;

var babelOptions = helper.merge({}, options, { filename: file.originalPath });
var processed = babel.transform(content, babelOptions);
var code = content;
if (processed) {
code = processed.code;
var cacheEntry = cache ? cache.getItem(file.originalPath) : null;
var md5 = crypto.createHash('md5').update(content).digest('hex');

if (!cacheEntry || cacheEntry.md5 !== md5) {
log.debug('Processing "%s" - Cache miss.', file.originalPath);

var babelOptions = helper.merge({}, options, { filename: file.originalPath });
var processed = babel.transform(content, babelOptions);
var code = content;
if (processed) {
code = processed.code;
}

if (cache) {
cache.setItem(file.originalPath, { md5: md5, code: code });
}
done(null, code);
} else {
log.debug('Processing "%s" - Cache hit.', file.originalPath);
done(null, cacheEntry.code);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function got a bit convoluted now. Also, the cache miss/hit messages are logged even when cache is disabled. Could you extract the caching logic to a function? For example:

var transformFunc = cache ? transformWithCache : transform;
var code = transformFunc(content, file);

function transform(content, file) {
  var babelOptions = helper.merge({}, options, { filename: file.originalPath });
  var processed = babel.transform(content, babelOptions);
  return processed ? processed.code : content;
}

function transformWithCache(content, file) {
  // Uses caching and the transform function.
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

}
done(null, code);
} catch (e) {
log.error('%s\n at %s', e.message, file.originalPath);
done(e, null);
Expand All @@ -34,10 +55,14 @@ function createPreprocessor(args, config, logger, helper) {
}

function createOptions(customConfig, baseConfig, helper, file) {
// Ignore 'base' property of custom preprocessor's config.
customConfig = helper.merge({}, customConfig);

// Ignore 'base' property of custom preprocessor's config.
delete customConfig.base;

// Ignore 'cachePath', which we use here instead of passing to babel.
delete baseConfig.cachePath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't mutate objects that this function doesn't create. Could you copy it and remove the property like it was/is done for base? Also, could you also do the same for customConfig?

Maybe you could define a helper function to copy a given object and remove given properties.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.


var customPerFile = createPerFileOptions(customConfig, helper, file);
var basePerFile = createPerFileOptions(baseConfig, helper, file);
var options = helper.merge(
Expand Down
Loading