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

Take dynamic dependencies into account when calculating hashes #2596

Merged
merged 1 commit into from Dec 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
35 changes: 20 additions & 15 deletions src/Chunk.ts
Expand Up @@ -669,13 +669,10 @@ export default class Chunk {
return (this.renderedHash = hash.digest('hex'));
}

/*
* Chunk dependency output graph post-visitor
* Visitor can return "true" to indicate a propagated stop condition
*/
postVisitChunkDependencies(visitor: (dep: Chunk | ExternalModule) => any): boolean {
visitStaticDependenciesUntilCondition(
isConditionSatisfied: (dep: Chunk | ExternalModule) => any
): boolean {
const seen = new Set<Chunk | ExternalModule>();
// add in hashes of all dependent chunks and resolved external ids
function visitDep(dep: Chunk | ExternalModule): boolean {
if (seen.has(dep)) return;
seen.add(dep);
Expand All @@ -684,24 +681,32 @@ export default class Chunk {
if (visitDep(subDep)) return true;
}
}
return visitor(dep) === true;
return isConditionSatisfied(dep) === true;
}
return visitDep(this);
}

visitDependencies(handleDependency: (dependency: Chunk | ExternalModule) => void) {
const toBeVisited: (Chunk | ExternalModule)[] = [this];
const visited: Set<Chunk | ExternalModule> = new Set();
for (const current of toBeVisited) {
handleDependency(current);
if (current instanceof ExternalModule) continue;
for (const dependency of current.dependencies.concat(current.dynamicDependencies)) {
if (!visited.has(dependency)) {
visited.add(dependency);
toBeVisited.push(dependency);
}
}
}
}

private computeContentHashWithDependencies(addons: Addons, options: OutputOptions): string {
const hash = sha256();

// own rendered source, except for finalizer wrapping
hash.update(this.getRenderedHash());
Copy link
Contributor

Choose a reason for hiding this comment

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

We do need the rendered hash of the source itself here. How else would changing the source without changing any other dependency information change the hash?

Copy link
Member Author

Choose a reason for hiding this comment

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

The rendered hash is added in visitDependencies. I made a note to add a new test category at some point to prove this is actually the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

It might be somewhat confusing, though, that visitDependencies always visits the base chunk first. Maybe the name should be improved.

hash.update(addons.hash);
hash.update(options.format);

// import names of dependency sources
hash.update(this.dependencies.length);

// add in hashes of all dependent chunks and resolved external ids
this.postVisitChunkDependencies(dep => {
this.visitDependencies(dep => {
if (dep instanceof ExternalModule) hash.update(':' + dep.renderPath);
else hash.update(dep.getRenderedHash());
});
Expand Down
8 changes: 4 additions & 4 deletions src/Module.ts
Expand Up @@ -38,7 +38,7 @@ import relativeId from './utils/relativeId';
import { RenderOptions } from './utils/renderHelpers';
import { SOURCEMAPPING_URL_RE } from './utils/sourceMappingURL';
import { timeEnd, timeStart } from './utils/timers';
import { visitStaticDependencies } from './utils/traverseStaticDependencies';
import { visitStaticModuleDependencies } from './utils/traverseStaticDependencies';
import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';

export interface CommentDescription {
Expand Down Expand Up @@ -416,8 +416,8 @@ export default class Module {
const name = isDefault
? 'default'
: isNamespace
? '*'
: (<ImportSpecifier>specifier).imported.name;
? '*'
: (<ImportSpecifier>specifier).imported.name;
this.importDescriptions[localName] = { source, start: specifier.start, name, module: null };
}
}
Expand All @@ -440,7 +440,7 @@ export default class Module {
includeAllExports() {
if (!this.isExecuted) {
this.graph.needsTreeshakingPass = true;
visitStaticDependencies(this, module => {
visitStaticModuleDependencies(this, module => {
if (module instanceof ExternalModule || module.isExecuted) return true;
module.isExecuted = true;
return false;
Expand Down
8 changes: 4 additions & 4 deletions src/chunk-optimization.ts
Expand Up @@ -19,7 +19,7 @@ export function optimizeChunks(
for (let chunkIndex = 0; chunkIndex < chunks.length; chunkIndex++) {
const mainChunk = chunks[chunkIndex];
const execGroup: Chunk[] = [];
mainChunk.postVisitChunkDependencies(dep => {
mainChunk.visitStaticDependenciesUntilCondition(dep => {
if (dep instanceof Chunk) {
execGroup.push(dep);
}
Expand Down Expand Up @@ -68,11 +68,11 @@ export function optimizeChunks(
// if (!chunk.isPure()) continue;

const chunkDependencies = new Set<Chunk | ExternalModule>();
chunk.postVisitChunkDependencies(dep => chunkDependencies.add(dep));
chunk.visitStaticDependenciesUntilCondition(dep => chunkDependencies.add(dep));

const ignoreSizeChunks = new Set<Chunk | ExternalModule>([chunk, lastChunk]);
if (
lastChunk.postVisitChunkDependencies(dep => {
lastChunk.visitStaticDependenciesUntilCondition(dep => {
if (dep === chunk || dep === lastChunk) {
return false;
}
Expand All @@ -96,7 +96,7 @@ export function optimizeChunks(
}

if (
chunk.postVisitChunkDependencies(dep => {
chunk.visitStaticDependenciesUntilCondition(dep => {
if (ignoreSizeChunks.has(dep)) {
return false;
}
Expand Down
6 changes: 3 additions & 3 deletions src/utils/traverseStaticDependencies.ts
@@ -1,11 +1,11 @@
import ExternalModule from '../ExternalModule';
import Module from '../Module';

export function visitStaticDependencies(
entryModule: Module | ExternalModule,
export function visitStaticModuleDependencies(
baseModule: Module | ExternalModule,
areDependenciesSkipped: (module: Module | ExternalModule) => boolean
) {
const modules = [entryModule];
const modules = [baseModule];
const visitedModules: { [id: string]: true } = {};
for (const module of modules) {
if (areDependenciesSkipped(module) || module instanceof ExternalModule) continue;
Expand Down
3 changes: 2 additions & 1 deletion test/chunking-form/index.js
Expand Up @@ -35,7 +35,8 @@ runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (di
extend(
{
dir: dir + '/_actual/' + format,
format
format,
chunkFileNames: 'generated-[name].js'
},
(config.options || {}).output || {}
),
Expand Down
5 changes: 1 addition & 4 deletions test/chunking-form/samples/aliasing-extensions/_config.js
@@ -1,9 +1,6 @@
module.exports = {
description: 'chunk aliasing with extensions',
options: {
input: ['main1', 'main2', 'main3.ts'],
output: {
chunkFileNames: 'generated-[name].js'
}
input: ['main1', 'main2', 'main3.ts']
}
};
@@ -1,4 +1,4 @@
define(['./chunk-96447c02.js'], function (__chunk_1) { 'use strict';
define(['./generated-chunk.js'], function (__chunk_1) { 'use strict';

function fn () {
console.log('dep1 fn');
Expand Down
@@ -1,4 +1,4 @@
define(['./chunk-96447c02.js'], function (__chunk_1) { 'use strict';
define(['./generated-chunk.js'], function (__chunk_1) { 'use strict';

function fn () {
console.log('lib1 fn');
Expand Down
@@ -1,6 +1,6 @@
'use strict';

var __chunk_1 = require('./chunk-ca3a1cbb.js');
var __chunk_1 = require('./generated-chunk.js');

function fn () {
console.log('dep1 fn');
Expand Down
@@ -1,6 +1,6 @@
'use strict';

var __chunk_1 = require('./chunk-ca3a1cbb.js');
var __chunk_1 = require('./generated-chunk.js');

function fn () {
console.log('lib1 fn');
Expand Down
@@ -1,4 +1,4 @@
import { a as fn } from './chunk-54f33655.js';
import { a as fn } from './generated-chunk.js';

function fn$1 () {
console.log('dep1 fn');
Expand Down
@@ -1,4 +1,4 @@
import { a as fn } from './chunk-54f33655.js';
import { a as fn } from './generated-chunk.js';

function fn$1 () {
console.log('lib1 fn');
Expand Down
@@ -1,4 +1,4 @@
System.register(['./chunk-9a21be24.js'], function (exports, module) {
System.register(['./generated-chunk.js'], function (exports, module) {
'use strict';
var fn;
return {
Expand Down
@@ -1,4 +1,4 @@
System.register(['./chunk-9a21be24.js'], function (exports, module) {
System.register(['./generated-chunk.js'], function (exports, module) {
'use strict';
var fn;
return {
Expand Down

This file was deleted.

@@ -0,0 +1,10 @@
define(['exports', './generated-chunk.js', './generated-chunk3.js'], function (exports, __chunk_1, __chunk_3) { 'use strict';

var x = __chunk_1.x + 1;

var y = __chunk_3.x + 1;

exports.x = x;
exports.y = y;

});
@@ -1,4 +1,4 @@
define(['./chunk-dcefc23d.js', './chunk-264eb6ba.js', './chunk-406f6d2a.js'], function (__chunk_1, __chunk_2, __chunk_3) { 'use strict';
define(['./generated-chunk.js', './generated-chunk2.js', './generated-chunk3.js'], function (__chunk_1, __chunk_2, __chunk_3) { 'use strict';

console.log(__chunk_2.x + __chunk_2.y);

Expand Down
@@ -1,4 +1,4 @@
define(['./chunk-dcefc23d.js', './chunk-264eb6ba.js', './chunk-406f6d2a.js'], function (__chunk_1, __chunk_2, __chunk_3) { 'use strict';
define(['./generated-chunk.js', './generated-chunk2.js', './generated-chunk3.js'], function (__chunk_1, __chunk_2, __chunk_3) { 'use strict';



Expand Down
@@ -1,4 +1,4 @@
define(['./chunk-dcefc23d.js'], function (__chunk_1) { 'use strict';
define(['./generated-chunk.js'], function (__chunk_1) { 'use strict';



Expand Down
@@ -1,4 +1,4 @@
define(['./chunk-406f6d2a.js'], function (__chunk_3) { 'use strict';
define(['./generated-chunk3.js'], function (__chunk_3) { 'use strict';



Expand Down

This file was deleted.

@@ -0,0 +1,11 @@
'use strict';

var __chunk_1 = require('./generated-chunk.js');
var __chunk_3 = require('./generated-chunk3.js');

var x = __chunk_1.x + 1;

var y = __chunk_3.x + 1;

exports.x = x;
exports.y = y;
@@ -1,7 +1,7 @@
'use strict';

require('./chunk-8a340a3c.js');
var __chunk_2 = require('./chunk-61a62bc6.js');
require('./chunk-4b766e46.js');
require('./generated-chunk.js');
var __chunk_2 = require('./generated-chunk2.js');
require('./generated-chunk3.js');

console.log(__chunk_2.x + __chunk_2.y);
@@ -1,6 +1,6 @@
'use strict';

require('./chunk-8a340a3c.js');
require('./chunk-61a62bc6.js');
require('./chunk-4b766e46.js');
require('./generated-chunk.js');
require('./generated-chunk2.js');
require('./generated-chunk3.js');

@@ -1,4 +1,4 @@
'use strict';

require('./chunk-8a340a3c.js');
require('./generated-chunk.js');

@@ -1,4 +1,4 @@
'use strict';

require('./chunk-4b766e46.js');
require('./generated-chunk3.js');

This file was deleted.

@@ -0,0 +1,8 @@
import { a as x } from './generated-chunk.js';
import { a as x$1 } from './generated-chunk3.js';

var x$2 = x + 1;

var y = x$1 + 1;

export { x$2 as a, y as b };
@@ -1,5 +1,5 @@
import './chunk-91999913.js';
import { a as x, b as y } from './chunk-c223c238.js';
import './chunk-78fb52ac.js';
import './generated-chunk.js';
import { a as x, b as y } from './generated-chunk2.js';
import './generated-chunk3.js';

console.log(x + y);
@@ -1,3 +1,3 @@
import './chunk-91999913.js';
import './chunk-c223c238.js';
import './chunk-78fb52ac.js';
import './generated-chunk.js';
import './generated-chunk2.js';
import './generated-chunk3.js';
@@ -1 +1 @@
import './chunk-91999913.js';
import './generated-chunk.js';
@@ -1 +1 @@
import './chunk-78fb52ac.js';
import './generated-chunk3.js';
@@ -1,4 +1,4 @@
System.register(['./chunk-f874c049.js', './chunk-6e722356.js'], function (exports, module) {
System.register(['./generated-chunk.js', './generated-chunk3.js'], function (exports, module) {
'use strict';
var x, x$1;
return {
Expand Down
@@ -1,4 +1,4 @@
System.register(['./chunk-f874c049.js', './chunk-61e8212c.js', './chunk-6e722356.js'], function (exports, module) {
System.register(['./generated-chunk.js', './generated-chunk2.js', './generated-chunk3.js'], function (exports, module) {
'use strict';
var x, y;
return {
Expand Down
@@ -1,4 +1,4 @@
System.register(['./chunk-f874c049.js', './chunk-61e8212c.js', './chunk-6e722356.js'], function (exports, module) {
System.register(['./generated-chunk.js', './generated-chunk2.js', './generated-chunk3.js'], function (exports, module) {
'use strict';
return {
setters: [function () {}, function () {}, function () {}],
Expand Down
@@ -1,4 +1,4 @@
System.register(['./chunk-f874c049.js'], function (exports, module) {
System.register(['./generated-chunk.js'], function (exports, module) {
'use strict';
return {
setters: [function () {}],
Expand Down
@@ -1,4 +1,4 @@
System.register(['./chunk-6e722356.js'], function (exports, module) {
System.register(['./generated-chunk3.js'], function (exports, module) {
'use strict';
return {
setters: [function () {}],
Expand Down

This file was deleted.

@@ -0,0 +1,5 @@
define(['./generated-chunk.js', './generated-chunk2.js'], function (__chunk_1, __chunk_2) { 'use strict';

console.log('11');

});
@@ -1,4 +1,4 @@
define(['./chunk-7754c915.js', './chunk-bea81d32.js', './chunk-7d7b15ed.js'], function (__chunk_1, __chunk_2, __chunk_3) { 'use strict';
define(['./generated-chunk.js', './generated-chunk2.js', './generated-chunk3.js'], function (__chunk_1, __chunk_2, __chunk_3) { 'use strict';

console.log('1');

Expand Down