Skip to content
This repository was archived by the owner on May 1, 2020. It is now read-only.

Commit fc44ca6

Browse files
authored
fix(uglify): verify source maps are generated correctly for all bundles, tests
* refactor(optimization): don't delete file from disk since we now write to memory don't delete file from disk since we now write to memory * test(babili): fix babili tests fix babili tests * needs tests but working * tests
1 parent 869455e commit fc44ca6

7 files changed

+181
-128
lines changed

config/uglifyjs.config.js

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,6 @@
33

44
module.exports = {
55

6-
/**
7-
* sourceFile: The javascript file to minify
8-
*/
9-
sourceFile: process.env.IONIC_OUTPUT_JS_FILE_NAME,
10-
11-
/**
12-
* destFileName: file name for the minified js in the build dir
13-
*/
14-
destFileName: process.env.IONIC_OUTPUT_JS_FILE_NAME,
15-
16-
/**
17-
* inSourceMap: file name for the input source map
18-
*/
19-
inSourceMap: process.env.IONIC_OUTPUT_JS_FILE_NAME + '.map',
20-
21-
/**
22-
* outSourceMap: file name for the output source map
23-
*/
24-
outSourceMap: process.env.IONIC_OUTPUT_JS_FILE_NAME + '.map',
25-
266
/**
277
* mangle: uglify 2's mangle option
288
*/

src/babili.spec.ts

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,47 +4,64 @@ import * as crossSpawn from 'cross-spawn';
44
import { EventEmitter } from 'events';
55

66
describe('babili function', () => {
7-
let emitter: EventEmitter;
87

98
beforeEach(() => {
10-
emitter = new EventEmitter();
119

12-
spyOn(configUtil, 'getUserConfigFile').and.returnValue('fileContents');
13-
spyOn(crossSpawn, 'spawn').and.returnValue(emitter);
10+
11+
1412
});
1513

16-
it('should call main babili function', () => {
14+
15+
16+
it('should reject promise when non-zero status code', () => {
17+
const spawnMock: any = {
18+
on: () => {}
19+
};
20+
spyOn(crossSpawn, 'spawn').and.returnValue(spawnMock);
21+
const onSpy = spyOn(spawnMock, 'on');
22+
1723
const context = {
18-
rootDir: '/Users/noone/Projects/ionic-conference-app'
24+
nodeModulesDir: '/Users/noone/Projects/ionic-conference-app/node_modules'
1925
};
20-
const configFile = 'configFileContents';
26+
const knownError = 'should never get here';
27+
28+
const promise = babili.runBabili(context);
29+
const spawnCallback = onSpy.calls.first().args[1];
30+
spawnCallback(1);
2131

22-
let pr = babili.babili(context, configFile);
23-
emitter.emit('close', 0);
24-
pr.then(() => {
25-
expect(configUtil.getUserConfigFile).toHaveBeenCalledWith(context, babili.taskInfo, configFile);
32+
return promise.then(() => {
33+
throw new Error(knownError);
34+
}).catch((err: Error) => {
35+
expect(err.message).not.toEqual(knownError);
2636
});
2737
});
2838

29-
it('should throw if context does not have a rootDir', () => {
30-
const context = {};
31-
const configFile = 'configFileContents';
39+
it('should resolve promise when zero status code', () => {
40+
const spawnMock: any = {
41+
on: () => {}
42+
};
43+
spyOn(crossSpawn, 'spawn').and.returnValue(spawnMock);
44+
const onSpy = spyOn(spawnMock, 'on');
3245

33-
expect(babili.babili(context, configFile)).toThrow();
34-
});
46+
const context = {
47+
nodeModulesDir: '/Users/noone/Projects/ionic-conference-app/node_modules'
48+
};
3549

36-
it('should fail because it does not have a valid build context', () => {
37-
const context: null = null;
38-
const configFile = 'configFileContents';
50+
const promise = babili.runBabili(context);
51+
const spawnCallback = onSpy.calls.first().args[1];
52+
spawnCallback(0);
3953

40-
expect(babili.babili(context, configFile)).toThrow();
54+
return promise;
4155
});
4256

43-
it('should fail because it does not have a valid config file', () => {
57+
it('should throw if context does not have a rootDir', () => {
4458
const context = {};
45-
const configFile: null = null;
46-
47-
expect(babili.babili(context, configFile)).toThrow();
59+
const knownError = 'should never get here';
60+
const promise = babili.runBabili(context);
61+
return promise.then(() => {
62+
throw new Error(knownError);
63+
}).catch((err: Error) => {
64+
expect(err.message).not.toEqual(knownError);
65+
});
4866
});
49-
5067
});

src/babili.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,16 @@ export function babili(context: BuildContext, configFile?: string) {
2323
export function babiliWorker(context: BuildContext, configFile: string) {
2424
const babiliConfig: BabiliConfig = fillConfigDefaults(configFile, taskInfo.defaultConfigFile);
2525
// TODO - figure out source maps??
26-
return runBabili(context, babiliConfig);
26+
return runBabili(context);
2727
}
2828

29-
function runBabili(context: BuildContext, config: BabiliConfig) {
30-
return runBabiliImpl(context);
31-
}
32-
33-
function runBabiliImpl(context: BuildContext) {
29+
export function runBabili(context: BuildContext) {
3430
// TODO - is there a better way to run this?
3531
return new Promise((resolve, reject) => {
36-
if (!context.rootDir) {
32+
if (!context.nodeModulesDir) {
3733
return reject(new Error('Babili failed because the context passed did not have a rootDir'));
3834
}
39-
const babiliPath = join(context.rootDir, 'node_modules', '.bin', 'babili');
35+
const babiliPath = join(context.nodeModulesDir, '.bin', 'babili');
4036
const command = spawn(babiliPath, [context.buildDir, '--out-dir', context.buildDir]);
4137
command.on('close', (code: number) => {
4238
if (code !== 0) {

src/optimization.spec.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { join } from 'path';
2+
13
import * as optimization from './optimization';
24
import * as decorators from './optimization/decorators';
35
import * as treeshake from './optimization/treeshake';
@@ -28,4 +30,35 @@ describe('optimization task', () => {
2830
expect(treeshake.calculateUnusedComponents).not.toHaveBeenCalled();
2931
});
3032
});
33+
34+
describe('purgeGeneratedFiles', () => {
35+
it('should remove files in buildDir with suffix from the cache', () => {
36+
const buildDir = '/some/fake/dir/myApp/www/build';
37+
const context = {
38+
fileCache: new FileCache(),
39+
buildDir: buildDir
40+
};
41+
const suffix = 'deptree.js';
42+
const filePathOne = join(buildDir, `0.${suffix}`);
43+
const filePathTwo = join(buildDir, `1.${suffix}`);
44+
const filePathThree = join(buildDir, `main.js`);
45+
const filePathFour = join(buildDir, `main.css`);
46+
const filePathFive = join('some', 'fake', 'dir', 'myApp', 'src', `app.ts`);
47+
const filePathSix = join('some', 'fake', 'dir', 'myApp', 'src', `app.js`);
48+
const filePathSeven = join('some', 'fake', 'dir', 'myApp', 'src', 'pages', `1.${suffix}`);
49+
context.fileCache.set(filePathOne, { path: filePathOne, content: filePathOne});
50+
context.fileCache.set(filePathTwo, { path: filePathTwo, content: filePathTwo});
51+
context.fileCache.set(filePathThree, { path: filePathThree, content: filePathThree});
52+
context.fileCache.set(filePathFour, { path: filePathFour, content: filePathFour});
53+
context.fileCache.set(filePathFive, { path: filePathFive, content: filePathFive});
54+
context.fileCache.set(filePathSix, { path: filePathSix, content: filePathSix});
55+
context.fileCache.set(filePathSeven, { path: filePathSeven, content: filePathSeven});
56+
57+
optimization.purgeGeneratedFiles(context, suffix);
58+
59+
expect(context.fileCache.getAll().length).toEqual(5);
60+
expect(context.fileCache.get(filePathOne)).toBeFalsy();
61+
expect(context.fileCache.get(filePathTwo)).toBeFalsy();
62+
});
63+
});
3164
});

src/optimization.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Logger } from './logger/logger';
33
import { fillConfigDefaults, getUserConfigFile, replacePathVars } from './util/config';
44
import * as Constants from './util/constants';
55
import { BuildError } from './util/errors';
6-
import { getBooleanPropertyValue, webpackStatsToDependencyMap, printDependencyMap, unlinkAsync } from './util/helpers';
6+
import { getBooleanPropertyValue, webpackStatsToDependencyMap, printDependencyMap } from './util/helpers';
77
import { BuildContext, TaskInfo } from './util/interfaces';
88
import { runWebpackFullBuild, WebpackConfig } from './webpack';
99
import { purgeDecorators } from './optimization/decorators';
@@ -32,7 +32,8 @@ function optimizationWorker(context: BuildContext, configFile: string): Promise<
3232
printDependencyMap(dependencyMap);
3333
Logger.debug('Original Dependency Map End');
3434
}
35-
return deleteOptimizationJsFile(join(webpackConfig.output.path, webpackConfig.output.filename));
35+
36+
purgeGeneratedFiles(context, webpackConfig.output.filename);
3637
}).then(() => {
3738
return doOptimizations(context, dependencyMap);
3839
});
@@ -41,8 +42,9 @@ function optimizationWorker(context: BuildContext, configFile: string): Promise<
4142
}
4243
}
4344

44-
export function deleteOptimizationJsFile(fileToDelete: string) {
45-
return unlinkAsync(fileToDelete);
45+
export function purgeGeneratedFiles(context: BuildContext, fileNameSuffix: string) {
46+
const buildFiles = context.fileCache.getAll().filter(file => file.path.indexOf(context.buildDir) >= 0 && file.path.indexOf(fileNameSuffix) >= 0);
47+
buildFiles.forEach(buildFile => context.fileCache.remove(buildFile.path));
4648
}
4749

4850
export function doOptimizations(context: BuildContext, dependencyMap: Map<string, Set<string>>) {

src/uglifyjs.spec.ts

Lines changed: 60 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,71 @@
1-
import * as uglifyjs from './uglifyjs';
2-
import * as configUtil from './util/config';
3-
import * as workerClient from './worker-client';
4-
5-
describe('uglifyjs function', () => {
6-
beforeEach(() => {
7-
spyOn(configUtil, 'getUserConfigFile').and.returnValue('fileContents');
8-
spyOn(workerClient, 'runWorker').and.returnValue(Promise.resolve());
9-
});
1+
import * as fs from 'fs';
2+
import { join } from 'path';
103

11-
it('should call workerClient function', () => {
12-
const context = {};
13-
const configFile = 'configFileContents';
4+
import * as uglifyLib from 'uglify-js';
145

15-
return uglifyjs.uglifyjs(context, configFile).then(() => {
16-
expect(configUtil.getUserConfigFile).toHaveBeenCalledWith(context, uglifyjs.taskInfo, configFile);
17-
expect(workerClient.runWorker).toHaveBeenCalledWith('uglifyjs', 'uglifyjsWorker', context, 'fileContents');
18-
});
19-
});
6+
import * as helpers from './util/helpers';
7+
import * as uglifyTask from './uglifyjs';
208

21-
it('should fail because it does not have a valid build context', () => {
22-
const context: null = null;
23-
const configFile = 'configFileContents';
249

25-
expect(uglifyjs.uglifyjs(context, configFile)).toThrow();
26-
});
10+
describe('uglifyjs', () => {
11+
describe('uglifyjsWorkerImpl', () => {
12+
it('should call uglify for the appropriate files', () => {
13+
const buildDir = join('some', 'fake', 'dir', 'myApp', 'www', 'build');
14+
const context = {
15+
buildDir: buildDir
16+
};
17+
const fileNames = ['polyfills.js', 'sw-toolbox.js', '0.main.js', '0.main.js.map', '1.main.js', '1.main.js.map', 'main.js', 'main.js.map'];
18+
const mockMinfiedResponse = {
19+
code: 'code',
20+
map: 'map'
21+
};
22+
const mockUglifyConfig = {
23+
mangle: true,
24+
compress: true
25+
};
2726

28-
it('should fail because it does not have a valid config file', () => {
29-
const context = {};
30-
const configFile: null = null;
27+
spyOn(fs, 'readdirSync').and.returnValue(fileNames);
28+
const uglifySpy = spyOn(uglifyLib, 'minify').and.returnValue(mockMinfiedResponse);
29+
const writeFileSpy = spyOn(helpers, helpers.writeFileAsync.name).and.returnValue(Promise.resolve());
3130

32-
expect(uglifyjs.uglifyjs(context, configFile)).toThrow();
33-
});
31+
const promise = uglifyTask.uglifyjsWorkerImpl(context, mockUglifyConfig);
32+
33+
return promise.then(() => {
34+
expect(uglifyLib.minify).toHaveBeenCalledTimes(3);
35+
expect(uglifySpy.calls.all()[0].args[0]).toEqual(join(buildDir, '0.main.js'));
36+
expect(uglifySpy.calls.all()[0].args[1].compress).toEqual(true);
37+
expect(uglifySpy.calls.all()[0].args[1].mangle).toEqual(true);
38+
expect(uglifySpy.calls.all()[0].args[1].inSourceMap).toEqual(join(buildDir, '0.main.js.map'));
39+
expect(uglifySpy.calls.all()[0].args[1].outSourceMap).toEqual(join(buildDir, '0.main.js.map'));
40+
41+
expect(uglifySpy.calls.all()[1].args[0]).toEqual(join(buildDir, '1.main.js'));
42+
expect(uglifySpy.calls.all()[1].args[1].compress).toEqual(true);
43+
expect(uglifySpy.calls.all()[1].args[1].mangle).toEqual(true);
44+
expect(uglifySpy.calls.all()[1].args[1].inSourceMap).toEqual(join(buildDir, '1.main.js.map'));
45+
expect(uglifySpy.calls.all()[1].args[1].outSourceMap).toEqual(join(buildDir, '1.main.js.map'));
46+
47+
expect(uglifySpy.calls.all()[2].args[0]).toEqual(join(buildDir, 'main.js'));
48+
expect(uglifySpy.calls.all()[2].args[1].compress).toEqual(true);
49+
expect(uglifySpy.calls.all()[2].args[1].mangle).toEqual(true);
50+
expect(uglifySpy.calls.all()[2].args[1].inSourceMap).toEqual(join(buildDir, 'main.js.map'));
51+
expect(uglifySpy.calls.all()[2].args[1].outSourceMap).toEqual(join(buildDir, 'main.js.map'));
52+
53+
expect(writeFileSpy).toHaveBeenCalledTimes(6);
54+
expect(writeFileSpy.calls.all()[0].args[0]).toEqual(join(buildDir, '0.main.js'));
55+
expect(writeFileSpy.calls.all()[0].args[1]).toEqual(mockMinfiedResponse.code);
56+
expect(writeFileSpy.calls.all()[1].args[0]).toEqual(join(buildDir, '0.main.js.map'));
57+
expect(writeFileSpy.calls.all()[1].args[1]).toEqual(mockMinfiedResponse.map);
3458

35-
it('should not fail if a config is not passed', () => {
36-
const context = {};
37-
let configFile: any;
59+
expect(writeFileSpy.calls.all()[2].args[0]).toEqual(join(buildDir, '1.main.js'));
60+
expect(writeFileSpy.calls.all()[2].args[1]).toEqual(mockMinfiedResponse.code);
61+
expect(writeFileSpy.calls.all()[3].args[0]).toEqual(join(buildDir, '1.main.js.map'));
62+
expect(writeFileSpy.calls.all()[3].args[1]).toEqual(mockMinfiedResponse.map);
3863

39-
return uglifyjs.uglifyjs(context).then(() => {
40-
expect(configUtil.getUserConfigFile).toHaveBeenCalledWith(context, uglifyjs.taskInfo, configFile);
41-
expect(workerClient.runWorker).toHaveBeenCalledWith('uglifyjs', 'uglifyjsWorker', context, 'fileContents');
64+
expect(writeFileSpy.calls.all()[4].args[0]).toEqual(join(buildDir, 'main.js'));
65+
expect(writeFileSpy.calls.all()[4].args[1]).toEqual(mockMinfiedResponse.code);
66+
expect(writeFileSpy.calls.all()[5].args[0]).toEqual(join(buildDir, 'main.js.map'));
67+
expect(writeFileSpy.calls.all()[5].args[1]).toEqual(mockMinfiedResponse.map);
68+
});
4269
});
4370
});
4471
});

0 commit comments

Comments
 (0)