add support for regex in cache paths #205
Conversation
|
Hi, @jameslnewell! I'm fine with this change, just need some tests for that to make sure config doesn't break, i.e. glob matching works the same with change and without. |
|
🆒 Thanks, will work on that now. Will there be any ramifications of not checking for absolute paths when using a regexp? https://github.com/jameslnewell/offline-plugin/blob/30802324b097ecdbed1cdd3a93f9f1823da045dd/src/index.js#L405 |
|
I think it should be something like this: (typeof cacheKey === 'string' || cacheKey instanceof RegExp) &&
!isAbsoluteURL(cacheKey) &&
cacheKey[0] !== '/' &&
cacheKey.indexOf('./') !== 0 &&
(magic = hasMagic(cacheKey))
|
|
|
|
@jameslnewell yeah, my bad, you got me confused about checking for absolute paths. There is a check for absolute url only to make sure that glob is valid and to not have false positives in I think it's mostly fine as you did now. Let's try this: let magic;
if (typeof cacheKey === 'string') {
magic =
!isAbsoluteURL(cacheKey) &&
cacheKey[0] !== '/' &&
cacheKey.indexOf('./') !== 0 &&
hasMagic(cacheKey);
} else if (cacheKey instanceof RegExp) {
magic = hasMagic(cacheKey);
} else {
// Ignore non-string and non-RegExp keys
return;
}
if (magic) {
// matching with magic...
} |
|
OK how's that? |
src/misc/utils.js
Outdated
| export { isAbsolutePath } |
There was a problem hiding this comment.
No line break in the end please here.
src/misc/utils.js
Outdated
| @@ -6,6 +6,14 @@ const isAbsolutePath = path.isAbsolute; | |||
| // Based on https://github.com/isaacs/node-glob/blob/master/glob.js#L83 | |||
| // (glob.hasMagic) | |||
| export function hasMagic(pattern, options) { | |||
|
|
|||
| //support RegExp as well as glob | |||
There was a problem hiding this comment.
// Support and remove empty line between function name and the comment please.
| caches: { | ||
| main: [/\.js$/] | ||
| }, | ||
| version: 'da39a3ee5e6b4b0d3255bfef95601890afd80709' |
docs/caches.md
Outdated
| @@ -8,7 +8,7 @@ ___________________________________ | |||
| Tells to the plugin what to cache and how. | |||
|
|
|||
| * `'all'`: means that everything (all the webpack output assets) and URLs listed in `externals` option will be cached on install. | |||
| * `Object`: Object with 3 possible sections (properties) of type `Array<string>`: `main`, `additional`, `optional`. All sections are optional and by default are empty (no assets added). | |||
| * `Object`: Object with 3 possible sections (properties) of type `Array<string|RegExp>`: `main`, `additional`, `optional`. All sections are optional and by default are empty (no assets added). | |||
docs/options.md
Outdated
| @@ -8,7 +8,7 @@ _Also see list of default options [here](https://github.com/NekR/offline-plugin/ | |||
| Allows you to define what to cache and how. | |||
|
|
|||
| * `'all'`: means that everything (all the webpack output assets) and URLs listed in `externals` option will be cached on install. | |||
| * `Object`: Object with 3 possible `Array<string>` sections (properties): `main`, `additional`, `optional`. All sections are optional and by default are empty (no assets added). | |||
| * `Object`: Object with 3 possible `Array<string|RegExp>` sections (properties): `main`, `additional`, `optional`. All sections are optional and by default are empty (no assets added). | |||
docs/caches.md
Outdated
| ``` |
There was a problem hiding this comment.
What is this change? Line break in the end?
There was a problem hiding this comment.
yeah soz, atom's adding them automatically
There was a problem hiding this comment.
oh, I see. I don't have any special feeling about trailing like breaks, I just don't like unnecessary/unrelated changes because who knows when you will need to merge things and why they could break then.
|
Tests fail. |
|
Okay, thank you @jameslnewell! I think I'll merge it tomorrow because I don't have time right for the final review. |
|
👍 thanks |
|
Okay, merging this, many thanks, @jameslnewell! (I'm not sure when will have a version out with this since all this changes go to v5 and there will be some delay before it's out) |
|
no problem |
Hi,
My app has files named like this:
client.[hash].jsclient.[id].[hash].jsI want to include
client.[hash].jsin mymaincache andclient.[id].[hash].jsin myoptionalcache. Using a glob makes thisdifficult(impossible?) because a glob likeclient.*.jsalways matches both files.I want to be able to use a
RegExpfor the cache configuration options in order to get around this problem e.g./client.[^.]+.js/