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

fix: restore stats output target functionality. #3030

Merged
merged 12 commits into from
Sep 24, 2021
6 changes: 6 additions & 0 deletions src/compiler/build/build-ctx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ export class BuildContext implements d.BuildCtx {
componentGraph = new Map<string, string[]>();
config: d.Config;
data: any = {};
buildStats?: d.CompilerBuildStats = undefined;
esmBrowserComponentBundle: d.BundleModule[];
esmComponentBundle: d.BundleModule[];
es5ComponentBundle: d.BundleModule[];
systemComponentBundle: d.BundleModule[];
commonJsComponentBundle: d.BundleModule[];
diagnostics: d.Diagnostic[] = [];
dirsAdded: string[] = [];
dirsDeleted: string[] = [];
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/build/build-finish.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type * as d from '../../declarations';
import { generateBuildResults } from './build-results';
import { generateBuildStats, writeBuildStats } from './build-stats';
import { isFunction, isRemoteUrl } from '@utils';
import { IS_NODE_ENV } from '../sys/environment';
import { relative } from 'path';
Expand All @@ -12,6 +13,7 @@ export const buildFinish = async (buildCtx: d.BuildCtx) => {
messages: buildCtx.buildMessages.slice(),
progress: 1,
};

buildCtx.compilerCtx.events.emit('buildLog', buildLog);

return results;
Expand All @@ -31,6 +33,12 @@ const buildDone = async (config: d.Config, compilerCtx: d.CompilerCtx, buildCtx:
// create the build results data
buildCtx.buildResults = generateBuildResults(config, compilerCtx, buildCtx);

// After the build results are available on the buildCtx, call the stats and set it.
// We will use this later to write the files.
buildCtx.buildStats = generateBuildStats(config, buildCtx);

await writeBuildStats(config, buildCtx.buildStats);

buildCtx.debug(`${aborted ? 'aborted' : 'finished'} build, ${buildCtx.buildResults.duration}ms`);

// log any errors/warnings
Expand Down
180 changes: 180 additions & 0 deletions src/compiler/build/build-stats.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
import { byteSize, sortBy } from '@utils';
import type * as d from '../../declarations';
import { isOutputTargetStats } from '../output-targets/output-utils';

/**
* Generates the Build Stats from the buildCtx. Writes any files to the file system.
* @param config the project build configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, the summary is missing in these JSDocs

* @param buildCtx An instance of the build which holds the details about the build
* @returns CompilerBuildStats or an Object including diagnostics.
*/
export function generateBuildStats(
config: d.Config,
buildCtx: d.BuildCtx
): d.CompilerBuildStats | { diagnostics: d.Diagnostic[] } {
const buildResults = buildCtx.buildResults;

let jsonData: d.CompilerBuildStats | { diagnostics: d.Diagnostic[] };

try {
if (buildResults.hasError) {
jsonData = {
diagnostics: buildResults.diagnostics,
};
} else {
const stats: d.CompilerBuildStats = {
timestamp: buildResults.timestamp,
compiler: {
name: config.sys.name,
version: config.sys.version,
},
app: {
namespace: config.namespace,
fsNamespace: config.fsNamespace,
components: Object.keys(buildResults.componentGraph).length,
entries: Object.keys(buildResults.componentGraph).length,
bundles: buildResults.outputs.reduce((total, en) => total + en.files.length, 0),
outputs: getAppOutputs(config, buildResults),
},
options: {
minifyJs: config.minifyJs,
minifyCss: config.minifyCss,
hashFileNames: config.hashFileNames,
hashedFileNameLength: config.hashedFileNameLength,
buildEs5: config.buildEs5,
},
formats: {
esmBrowser: sanitizeBundlesForStats(buildCtx.esmBrowserComponentBundle),
esm: sanitizeBundlesForStats(buildCtx.esmComponentBundle),
es5: sanitizeBundlesForStats(buildCtx.es5ComponentBundle),
system: sanitizeBundlesForStats(buildCtx.systemComponentBundle),
commonjs: sanitizeBundlesForStats(buildCtx.commonJsComponentBundle),
},
components: getComponentsFileMap(config, buildCtx),
entries: buildCtx.entryModules,
componentGraph: buildResults.componentGraph,
sourceGraph: getSourceGraph(config, buildCtx),
rollupResults: buildCtx.rollupResults,
collections: getCollections(config, buildCtx),
};

jsonData = stats;
}
} catch (e) {
jsonData = {
diagnostics: [e.message],
};
}

return jsonData;
}

/**
* Writes the files from the stats config to the file system
* @param config the project build configuration
* @param buildCtx An instance of the build which holds the details about the build
* @returns
*/
export async function writeBuildStats(config: d.Config, data: d.CompilerBuildStats | { diagnostics: d.Diagnostic[] }) {
const statsTargets = config.outputTargets.filter(isOutputTargetStats);

await Promise.all(
statsTargets.map(async (outputTarget) => {
const result = await config.sys.writeFile(outputTarget.file, JSON.stringify(data, null, 2));

if (result.error) {
config.logger.warn([`Stats failed to write file to ${outputTarget.file}`]);
}
})
);
}

function sanitizeBundlesForStats(bundleArray: ReadonlyArray<d.BundleModule>): ReadonlyArray<d.CompilerBuildStatBundle> {
if (!bundleArray) {
return [];
}

return bundleArray.map((bundle) => {
return {
key: bundle.entryKey,
components: bundle.cmps.map((c) => c.tagName),
bundleId: bundle.output.bundleId,
fileName: bundle.output.fileName,
imports: bundle.rollupResult.imports,
// code: bundle.rollupResult.code, // (use this to debug)
// Currently, this number is inaccurate vs what seems to be on disk.
originalByteSize: byteSize(bundle.rollupResult.code),
};
});
}

function getSourceGraph(config: d.Config, buildCtx: d.BuildCtx) {
let sourceGraph: d.BuildSourceGraph = {};

sortBy(buildCtx.moduleFiles, (m) => m.sourceFilePath).forEach((moduleFile) => {
const key = relativePath(config, moduleFile.sourceFilePath);
sourceGraph[key] = moduleFile.localImports.map((localImport) => relativePath(config, localImport)).sort();
});

return sourceGraph;
}

function getAppOutputs(config: d.Config, buildResults: d.CompilerBuildResults) {
return buildResults.outputs.map((output) => {
return {
name: output.type,
files: output.files.length,
generatedFiles: output.files.map((file) => relativePath(config, file)),
};
});
}

function getComponentsFileMap(config: d.Config, buildCtx: d.BuildCtx) {
return buildCtx.components.map((component) => {
return {
tag: component.tagName,
path: relativePath(config, component.jsFilePath),
source: relativePath(config, component.sourceFilePath),
elementRef: component.elementRef,
componentClassName: component.componentClassName,
assetsDirs: component.assetsDirs,
dependencies: component.dependencies,
dependents: component.dependents,
directDependencies: component.directDependencies,
directDependents: component.directDependents,
docs: component.docs,
encapsulation: component.encapsulation,
excludeFromCollection: component.excludeFromCollection,
events: component.events,
internal: component.internal,
legacyConnect: component.legacyConnect,
legacyContext: component.legacyContext,
listeners: component.listeners,
methods: component.methods,
potentialCmpRefs: component.potentialCmpRefs,
properties: component.properties,
shadowDelegatesFocus: component.shadowDelegatesFocus,
states: component.states,
};
});
}

function getCollections(config: d.Config, buildCtx: d.BuildCtx) {
return buildCtx.collections
.map((c) => {
return {
name: c.collectionName,
source: relativePath(config, c.moduleDir),
tags: c.moduleFiles.map((m) => m.cmps.map((cmp: d.ComponentCompilerMeta) => cmp.tagName)).sort(),
};
})
.sort((a, b) => {
if (a.name < b.name) return -1;
if (a.name > b.name) return 1;
return 0;
});
}

function relativePath(config: d.Config, file: string) {
return config.sys.normalizePath(config.sys.platformPath.relative(config.rootDir, file));
}
63 changes: 63 additions & 0 deletions src/compiler/build/test/build-stats.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import type * as d from '@stencil/core/declarations';
import { mockConfig, mockCompilerCtx, mockBuildCtx } from '@stencil/core/testing';
import { generateBuildResults } from '../build-results';
import { generateBuildStats } from '../build-stats';
import path from 'path';

describe('generateBuildStats', () => {
const root = path.resolve('/');
const config = mockConfig();
let compilerCtx: d.CompilerCtx;
let buildCtx: d.BuildCtx;

beforeEach(() => {
compilerCtx = mockCompilerCtx(config);
buildCtx = mockBuildCtx(config, compilerCtx);
});

it('should return a structured json object', async () => {
buildCtx.buildResults = generateBuildResults(config, compilerCtx, buildCtx);

const result: d.CompilerBuildStats = generateBuildStats(config, buildCtx);

if (result.hasOwnProperty('timestamp')) {
delete result.timestamp;
}

if (result.hasOwnProperty('compiler') && result.compiler.hasOwnProperty('version')) {
delete result.compiler.version;
}
Comment on lines +27 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

What value was this producing that we had to delete it and remove it from the assertion below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version that's built at the moment of the test's build (it appended the date and time to the version)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, what was it that we changed here that made this suddenly fail?

Copy link
Contributor Author

@splitinfinities splitinfinities Sep 22, 2021

Choose a reason for hiding this comment

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

It had been failing all along, actually, I just hadn't realized it because I didn't look into the build task on Github. My local build version wasn't failing because the timestamp hadn't changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess more concretely, what did we change in this PR to cause this to fail? Or are you saying it was failing since we turned tests back on in CI?

Copy link
Contributor Author

@splitinfinities splitinfinities Sep 24, 2021

Choose a reason for hiding this comment

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

I mean quite literally that all the tests in all of my previous commits failed on CI because the built compiler's version includes a timestamp, so I deleted the version that includes the timestamp so I don't have to test the object against that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. Arguably, we should mock timestamp out, but I think that may be out of scope for this PR. Would you mind creating a ticket and adding it to the backlog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

STENCIL-137


expect(result).toStrictEqual({
app: { bundles: 0, components: 0, entries: 0, fsNamespace: undefined, namespace: 'Testing', outputs: [] },
collections: [],
compiler: { name: 'in-memory' },
componentGraph: {},
components: [],
entries: [],
formats: { commonjs: [], es5: [], esm: [], esmBrowser: [], system: [] },
options: {
buildEs5: false,
hashFileNames: false,
hashedFileNameLength: undefined,
minifyCss: false,
minifyJs: false,
},
rollupResults: undefined,
sourceGraph: {},
});
});

it('should return diagnostics if an error is hit', async () => {
buildCtx.buildResults = generateBuildResults(config, compilerCtx, buildCtx);

buildCtx.buildResults.hasError = true;
buildCtx.buildResults.diagnostics = ['Something bad happened'];

const result = generateBuildStats(config, buildCtx);

expect(result).toStrictEqual({
diagnostics: ['Something bad happened'],
});
});
});
2 changes: 1 addition & 1 deletion src/compiler/entries/component-graph.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type * as d from '../../declarations';
import { getScopeId } from '../style/scope-css';

export const generateModuleGraph = (cmps: d.ComponentCompilerMeta[], bundleModules: d.BundleModule[]) => {
export const generateModuleGraph = (cmps: d.ComponentCompilerMeta[], bundleModules: ReadonlyArray<d.BundleModule>) => {
const cmpMap = new Map<string, string[]>();
cmps.forEach((cmp) => {
const bundle = bundleModules.find((b) => b.cmps.includes(cmp));
Expand Down
10 changes: 7 additions & 3 deletions src/compiler/output-targets/dist-lazy/generate-cjs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const generateCjs = async (
buildCtx: d.BuildCtx,
rollupBuild: RollupBuild,
outputTargets: d.OutputTargetDistLazy[]
) => {
): Promise<d.UpdatedBuildCtx> => {
const cjsOutputs = outputTargets.filter((o) => !!o.cjsDir);

if (cjsOutputs.length > 0) {
Expand All @@ -25,7 +25,8 @@ export const generateCjs = async (
const results = await generateRollupOutput(rollupBuild, esmOpts, config, buildCtx.entryModules);
if (results != null) {
const destinations = cjsOutputs.map((o) => o.cjsDir);
await generateLazyModules(

buildCtx.commonJsComponentBundle = await generateLazyModules(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bundles here that were produced during a build were not available at any point during the process, and the result would get tossed. I've modified BuildContext to allow for stats to be stored on all instances of the class.

The return signature of this has changed as well, providing back an updated buildCtx.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we update the signature of the function to reflect we return something please? (and in the other files where we generate other module outputs for lazy)?

config,
compilerCtx,
buildCtx,
Expand All @@ -36,16 +37,19 @@ export const generateCjs = async (
false,
'.cjs'
);

await generateShortcuts(compilerCtx, results, cjsOutputs);
}
}

return { name: 'cjs', buildCtx };
};

const generateShortcuts = (
compilerCtx: d.CompilerCtx,
rollupResult: d.RollupResult[],
outputTargets: d.OutputTargetDistLazy[]
) => {
): Promise<void[]> => {
const indexFilename = rollupResult.find((r) => r.type === 'chunk' && r.isIndex).fileName;
return Promise.all(
outputTargets.map(async (o) => {
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/output-targets/dist-lazy/generate-esm-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const generateEsmBrowser = async (
buildCtx: d.BuildCtx,
rollupBuild: RollupBuild,
outputTargets: d.OutputTargetDistLazy[]
) => {
): Promise<d.UpdatedBuildCtx> => {
const esmOutputs = outputTargets.filter((o) => !!o.esmDir && !!o.isBrowserBuild);
if (esmOutputs.length) {
const outputTargetType = esmOutputs[0].type;
Expand All @@ -29,7 +29,7 @@ export const generateEsmBrowser = async (

if (output != null) {
const es2017destinations = esmOutputs.map((o) => o.esmDir);
const componentBundle = await generateLazyModules(
buildCtx.esmBrowserComponentBundle = await generateLazyModules(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar scenario here as in src/compiler/output-targets/dist-lazy/generate-cjs.ts

config,
compilerCtx,
buildCtx,
Expand All @@ -40,8 +40,8 @@ export const generateEsmBrowser = async (
true,
''
);
return componentBundle;
}
}
return undefined;

return { name: 'esm-browser', buildCtx };
};
Loading