Skip to content
This repository has been archived by the owner on Nov 20, 2019. It is now read-only.

feat: add assetsHash for cache naming #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: add assetsHash for cache naming #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee67924
feat: add assetsHash for cache naming
nename0 Sep 23, 2018
cce46b9
test: make test check assetsHash value
nename0 Sep 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import path from 'path'
import SingleEntryPlugin from 'webpack/lib/SingleEntryPlugin'
import createHash from 'webpack/lib/util/createHash'
import minimatch from 'minimatch'

function validatePaths(assets, options) {
Expand All @@ -24,6 +25,18 @@ function validatePaths(assets, options) {
})
}

function hash(source, outputOptions) {
const hashFunction = outputOptions.hashFunction
const hashDigest = outputOptions.hashDigest
const hashDigestLength = outputOptions.hashDigestLength
const hash = createHash(hashFunction)
if (outputOptions.hashSalt) {
hash.update(outputOptions.hashSalt)
}
hash.update(source)
return hash.digest(hashDigest).substr(0, hashDigestLength)
}

const COMPILER_NAME = 'serviceworker-plugin'

export default class ServiceWorkerPlugin {
Expand Down Expand Up @@ -176,8 +189,11 @@ export default class ServiceWorkerPlugin {

assets = validatePaths(assets, this.options)

const assetsHash = hash(JSON.stringify(assets), compilation.options.output)

const serviceWorkerOption = this.options.transformOptions({
assets,
assetsHash,
jsonStats,
})

Expand Down
82 changes: 78 additions & 4 deletions src/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ function trim(str) {

const filename = 'sw.js'
const webpackOutputPath = path.resolve('./tmp-build')
const outputOptions = {
path: webpackOutputPath,
// these are the defaults from https://github.com/webpack/webpack/blob/master/lib/WebpackOptionsDefaulter.js#L164
hashFunction: 'md4',
hashDigest: 'hex',
hashDigestLength: 20,
}
const makeWebpackConfig = options => ({
entry: path.join(__dirname, '../test/test-build-entry'),
mode: 'development',
Expand All @@ -29,10 +36,9 @@ const makeWebpackConfig = options => ({
'serviceworker-webpack-plugin/lib/runtime': path.join(__dirname, 'runtime.js'),
},
},
output: {
path: webpackOutputPath,
},
output: outputOptions,
})
const fakeOptions = { output: outputOptions }

describe('ServiceWorkerPlugin', () => {
beforeEach(done => {
Expand Down Expand Up @@ -96,6 +102,7 @@ describe('ServiceWorkerPlugin', () => {
})

const compilation = {
options: fakeOptions,
assets: {
[filename]: {
source: () => '',
Expand Down Expand Up @@ -140,6 +147,7 @@ var serviceWorkerOption = {
})

const compilation = {
options: fakeOptions,
assets: {
[filename]: {
source: () => '',
Expand Down Expand Up @@ -170,7 +178,73 @@ var serviceWorkerOption = {
)
}
)
})
}),
it('should get passed assetsHash', done => {
const transformOptions = serviceWorkerOption => {
expect(serviceWorkerOption)
.to.have.property('assetsHash')
.that.is.equal('bd90271e0847dcc66adb')
}

const serviceWorkerPlugin = new ServiceWorkerPlugin({
filename,
transformOptions,
})

const compilation = {
options: fakeOptions,
assets: {
[filename]: {
source: () => '',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps you can add a second test case, with a different mocked source, as evidence that the hash is something else when the contents change.

Copy link
Author

@nename0 nename0 Sep 24, 2018 •
edited

Choose a reason for hiding this comment

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

Oh I think we've a design decision here. Maybe I didn't explain fully, but currently only the filenames get hashed not the contents. In my project I use filenames with hashes so there is no problem with this. On the other hand after reading this I think the only real use case to make the assetsHash include the content would be to make the serviceworker byte different when the assets change, but not their filenames. In this case it would probably be easier to just make the jsonStats include the compilation hash by changing this line to true.
What do you think @woutervanvliet ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Think I indeed misunderstood the point of your PR, but yes - my approach would be to have some kind of value, that's the same between builds of the same code, but different when sourcecode changes.

Tbh; I'm not that familiar with webpack internals, but it sounds like enabling the hash would solve the same problem as your PR does.

@devCrossNet @oliviertassinari What do you say?

},
'bar-v1.js': {},
},
getStats: () => ({
toJson: () => ({}),
}),
}

return serviceWorkerPlugin.handleEmit(
compilation,
{
options: {},
},
done
)
}),
it('should change assetsHash when filename changes', done => {
const transformOptions = serviceWorkerOption => {
expect(serviceWorkerOption)
.to.have.property('assetsHash')
.that.is.equal('47633aad38a6adaca6ec')
}

const serviceWorkerPlugin = new ServiceWorkerPlugin({
filename,
transformOptions,
})

const compilation = {
options: fakeOptions,
assets: {
[filename]: {
source: () => '',
},
'bar-v2.js': {},
},
getStats: () => ({
toJson: () => ({}),
}),
}

return serviceWorkerPlugin.handleEmit(
compilation,
{
options: {},
},
done
)
})
})
})
})