-
Notifications
You must be signed in to change notification settings - Fork 795
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
Changes from 4 commits
aa7fd72
f05890b
da2e392
a58bc3c
d0f7be8
907f105
e8e662a
4ff5749
d1e73fe
807caa9
d5bc274
303af48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
import { byteSize, sortBy } from '@utils'; | ||
import type * as d from '../../declarations'; | ||
|
||
/** | ||
* | ||
* @param config | ||
* @param compilerCtx | ||
* @param buildCtx | ||
* @returns | ||
*/ | ||
export function generateBuildStats(config: d.Config, buildCtx: d.BuildCtx) { | ||
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), | ||
rwaskiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; | ||
} | ||
|
||
/** | ||
* | ||
* @param config | ||
* @param buildCtx | ||
* @returns | ||
*/ | ||
export async function writeBuildStats(config: d.Config, data: d.CompilerBuildStats) { | ||
const statsTargets = config.outputTargets.filter((o) => o.type === 'stats') as d.OutputTargetStats[]; | ||
rwaskiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
await Promise.all( | ||
statsTargets.map((outputTarget) => { | ||
config.sys.writeFile(outputTarget.file, JSON.stringify(data, null, 2)); | ||
rwaskiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
); | ||
} | ||
|
||
function sanitizeBundlesForStats(bundleArray: d.BundleModule[]): d.CompilerBuildStatBundle[] { | ||
rwaskiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!bundleArray) { | ||
return []; | ||
} | ||
|
||
return bundleArray.map((bundle) => { | ||
return { | ||
// Should probably get the file size too. | ||
rwaskiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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. | ||
byteSize: byteSize(bundle.rollupResult.code), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rwaskiewicz or @Lars, any recs about how to get this number to be more accurate without reading the file from disk? Should I omit for the time being? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How inaccurate are we talking? Like an order of magnitude? or a few bytes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, duh. It's because the files are compressed. It's a symptom of uglifying the files being applied after the build. Ironically, there's also some larger discrepancy between esmBrowser and esm as a result of esm not being minified. Since esm is not compressed right now (there's another bug somewhere filed about this...) the esm file's byte sizes produced at this point are accurate. I think I'm going to rename this to "originalByteSize" instead. We should in the future figure out how to add that. Feels like an ordering problem. I'd hate to read files, as generating the stats may be much slower. |
||
}; | ||
}); | ||
} | ||
|
||
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)); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
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 = generateBuildStats(config, buildCtx); | ||
|
||
if (result.hasOwnProperty('timestamp')) { | ||
delete result.timestamp; | ||
} | ||
|
||
expect(result).toStrictEqual({ | ||
app: { bundles: 0, components: 0, entries: 0, fsNamespace: undefined, namespace: 'Testing', outputs: [] }, | ||
collections: [], | ||
compiler: { name: 'in-memory', version: '0.0.0-dev.20210827160156' }, | ||
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'], | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
@@ -36,9 +37,12 @@ export const generateCjs = async ( | |
false, | ||
'.cjs' | ||
); | ||
|
||
await generateShortcuts(compilerCtx, results, cjsOutputs); | ||
} | ||
} | ||
|
||
return buildCtx; | ||
}; | ||
|
||
const generateShortcuts = ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar scenario here as in |
||
config, | ||
compilerCtx, | ||
buildCtx, | ||
|
@@ -40,8 +40,7 @@ export const generateEsmBrowser = async ( | |
true, | ||
'' | ||
); | ||
return componentBundle; | ||
} | ||
} | ||
return undefined; | ||
return buildCtx; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,9 +24,10 @@ export const generateEsm = async ( | |
}; | ||
const outputTargetType = esmOutputs[0].type; | ||
const output = await generateRollupOutput(rollupBuild, esmOpts, config, buildCtx.entryModules); | ||
|
||
if (output != null) { | ||
const es2017destinations = esmOutputs.map((o) => o.esmDir); | ||
await generateLazyModules( | ||
buildCtx.esmComponentBundle = await generateLazyModules( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar scenario here as in |
||
config, | ||
compilerCtx, | ||
buildCtx, | ||
|
@@ -39,7 +40,7 @@ export const generateEsm = async ( | |
); | ||
|
||
const es5destinations = esmEs5Outputs.map((o) => o.esmEs5Dir); | ||
await generateLazyModules( | ||
buildCtx.es5ComponentBundle = await generateLazyModules( | ||
config, | ||
compilerCtx, | ||
buildCtx, | ||
|
@@ -55,6 +56,8 @@ export const generateEsm = async ( | |
await generateShortcuts(config, compilerCtx, outputTargets, output); | ||
} | ||
} | ||
|
||
return buildCtx; | ||
}; | ||
|
||
const copyPolyfills = async (config: d.Config, compilerCtx: d.CompilerCtx, outputTargets: d.OutputTargetDistLazy[]) => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ export const generateSystem = async ( | |
const results = await generateRollupOutput(rollupBuild, esmOpts, config, buildCtx.entryModules); | ||
if (results != null) { | ||
const destinations = systemOutputs.map((o) => o.esmDir); | ||
await generateLazyModules( | ||
buildCtx.systemComponentBundle = await generateLazyModules( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar scenario to |
||
config, | ||
compilerCtx, | ||
buildCtx, | ||
|
@@ -37,9 +37,12 @@ export const generateSystem = async ( | |
true, | ||
'.system' | ||
); | ||
|
||
await generateSystemLoaders(config, compilerCtx, results, systemOutputs); | ||
} | ||
} | ||
|
||
return buildCtx; | ||
}; | ||
|
||
const generateSystemLoaders = ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,15 +59,13 @@ export const outputLazy = async (config: d.Config, compilerCtx: d.CompilerCtx, b | |
|
||
const rollupBuild = await bundleOutput(config, compilerCtx, buildCtx, bundleOpts); | ||
if (rollupBuild != null) { | ||
const [componentBundle] = await Promise.all([ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Within this promise, the return signature in generateEsmBrowser was used and the others returned undefined. Refactoring to reset the buildCtx where the function does internal modifications felt like a better fit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will effectively serializes these outputs - do we have any measurements on how that impacts performance of the lazy build? If not, can we generate some? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lars generated some, but I'll write a node script to run a build 10 times to gather and report data. |
||
generateEsmBrowser(config, compilerCtx, buildCtx, rollupBuild, outputTargets), | ||
generateEsm(config, compilerCtx, buildCtx, rollupBuild, outputTargets), | ||
generateSystem(config, compilerCtx, buildCtx, rollupBuild, outputTargets), | ||
generateCjs(config, compilerCtx, buildCtx, rollupBuild, outputTargets), | ||
]); | ||
buildCtx = await generateEsmBrowser(config, compilerCtx, buildCtx, rollupBuild, outputTargets); | ||
buildCtx = await generateEsm(config, compilerCtx, buildCtx, rollupBuild, outputTargets); | ||
buildCtx = await generateSystem(config, compilerCtx, buildCtx, rollupBuild, outputTargets); | ||
buildCtx = await generateCjs(config, compilerCtx, buildCtx, rollupBuild, outputTargets); | ||
|
||
if (componentBundle != null) { | ||
buildCtx.componentGraph = generateModuleGraph(buildCtx.components, componentBundle); | ||
if (buildCtx.esmBrowserComponentBundle != null) { | ||
buildCtx.componentGraph = generateModuleGraph(buildCtx.components, buildCtx.esmBrowserComponentBundle); | ||
} | ||
} | ||
} catch (e) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, the JSDocs in this file just seem to be stubbed out