Skip to content

Commit 551b825

Browse files
committed
Refactor doCompile() to avoid content mutation
1 parent 003a007 commit 551b825

File tree

1 file changed

+26
-20
lines changed

1 file changed

+26
-20
lines changed

packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.ts

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -217,18 +217,25 @@ export class DataSchemaCompiler {
217217
this.pythonContext = await this.loadPythonContext(files, 'globals.py');
218218
this.yamlCompiler.initFromPythonContext(this.pythonContext);
219219

220-
// As we mutate files data, we need a copy, not a refs.
221-
// FileContent is a plain object with primitives, so it's enough for a shallow copy.
222-
let toCompile = (this.filesToCompile?.length
220+
const originalJsFiles: FileContent[] = [];
221+
const jinjaTemplatedFiles: FileContent[] = [];
222+
const yamlTemplatedFiles: FileContent[] = [];
223+
224+
(this.filesToCompile?.length
223225
? files.filter(f => this.filesToCompile.includes(f.fileName))
224-
: files).filter(f => f.fileName.endsWith('.js')
225-
// We don't transpile/compile other files (like .py and so on)
226-
|| f.fileName.endsWith('.yml')
227-
|| f.fileName.endsWith('.yaml')
228-
|| f.fileName.endsWith('.jinja')).map(f => ({ ...f }));
226+
: files).forEach(file => {
227+
if (file.fileName.endsWith('.js')) {
228+
originalJsFiles.push(file);
229+
} else if (file.fileName.endsWith('.jinja') ||
230+
(file.fileName.endsWith('.yml') || file.fileName.endsWith('.yaml')) && file.content.match(JINJA_SYNTAX)) {
231+
jinjaTemplatedFiles.push(file);
232+
} else if (file.fileName.endsWith('.yml') || file.fileName.endsWith('.yaml')) {
233+
yamlTemplatedFiles.push(file);
234+
}
235+
// We don't transpile/compile other files (like .py and so on)
236+
});
229237

230-
const jinjaTemplatedFiles = toCompile.filter((file) => file.fileName.endsWith('.jinja') ||
231-
(file.fileName.endsWith('.yml') || file.fileName.endsWith('.yaml')) && file.content.match(JINJA_SYNTAX));
238+
let toCompile = [...jinjaTemplatedFiles, ...yamlTemplatedFiles, ...originalJsFiles];
232239

233240
if (jinjaTemplatedFiles.length > 0) {
234241
// Preload Jinja templates to the engine
@@ -422,6 +429,13 @@ export class DataSchemaCompiler {
422429
asyncModules = [];
423430
transpiledFiles = await transpile(stage);
424431

432+
if (stage === 0) {
433+
// We render jinja and transpile yaml only once on first phase and then use resulting JS for these files
434+
// afterward avoiding costly YAML/Python parsing again. Original JS files are preserved as is for cache hits.
435+
const convertedToJsFiles = transpiledFiles.filter(f => f.convertedToJs);
436+
toCompile = [...originalJsFiles, ...convertedToJsFiles];
437+
}
438+
425439
return this.compileCubeFiles(cubes, contexts, compiledFiles, asyncModules, compilers, transpiledFiles, errorsReport);
426440
};
427441

@@ -512,23 +526,15 @@ export class DataSchemaCompiler {
512526
this.pythonContext!
513527
);
514528
if (transpiledFile) {
515-
// We update the jinja/yaml file content to the transpiled js content
516-
// and raise related flag so it will go JS transpilation flow afterward
517-
// avoiding costly YAML/Python parsing again.
518-
file.content = transpiledFile.content;
519-
file.convertedToJs = true;
529+
transpiledFile.convertedToJs = true;
520530
}
521531

522532
return transpiledFile;
523533
} else if (file.fileName.endsWith('.yml') || file.fileName.endsWith('.yaml')) {
524534
const transpiledFile = this.yamlCompiler.transpileYamlFile(file, errorsReport);
525535

526536
if (transpiledFile) {
527-
// We update the yaml file content to the transpiled js content
528-
// and raise related flag so it will go JS transpilation flow afterward
529-
// avoiding costly YAML/Python parsing again.
530-
file.content = transpiledFile.content;
531-
file.convertedToJs = true;
537+
transpiledFile.convertedToJs = true;
532538
}
533539

534540
return transpiledFile;

0 commit comments

Comments
 (0)