Skip to content

Commit 8481b9c

Browse files
committed
Fix recursive loops
1 parent e04a932 commit 8481b9c

File tree

3 files changed

+168
-142
lines changed

3 files changed

+168
-142
lines changed

packages/app/src/sandbox/eval/manager.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ export default class Manager {
123123
fileResolver: IFileResolver | undefined;
124124

125125
// List of modules that are being transpiled, to prevent duplicate jobs.
126-
transpileJobs: { [transpiledModuleId: string]: true };
126+
transpileJobs: {
127+
[transpiledModuleId: string]: true | Promise<TranspiledModule>;
128+
};
129+
127130
transpiledModulesByHash: { [hash: string]: TranspiledModule };
128131

129132
// All paths are resolved at least twice: during transpilation and evaluation.

packages/app/src/sandbox/eval/transpiled-module.ts

Lines changed: 162 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import evaluate from './loaders/eval';
2121
import Manager, { HMRStatus } from './manager';
2222
import HMR from './hmr';
2323
import { splitQueryFromPath } from './utils/query-path';
24+
import delay from '../utils/delay';
2425

2526
declare const BrowserFS: any;
2627

@@ -564,169 +565,190 @@ export default class TranspiledModule {
564565
* after the initial transpilation finished.
565566
* @param {*} manager
566567
*/
567-
async transpile(manager: Manager) {
568-
try {
569-
if (this.source) {
570-
return this;
571-
}
568+
async doTranspile(manager: Manager) {
569+
// eslint-disable-next-line
570+
manager.transpileJobs[this.getId()] = true;
572571

573-
if (manager.transpileJobs[this.getId()]) {
574-
// Is already being transpiled
575-
return this;
576-
}
577-
578-
// eslint-disable-next-line
579-
manager.transpileJobs[this.getId()] = true;
572+
this.hasMissingDependencies = false;
580573

581-
this.hasMissingDependencies = false;
574+
// Remove this module from the initiators of old deps, so we can populate a
575+
// fresh cache
576+
this.dependencies.forEach(tModule => {
577+
tModule.initiators.delete(this);
578+
});
579+
this.transpilationDependencies.forEach(tModule => {
580+
tModule.transpilationInitiators.delete(this);
581+
});
582+
this.childModules.forEach(tModule => {
583+
tModule.dispose(manager);
584+
});
585+
this.dependencies.clear();
586+
this.transpilationDependencies.clear();
587+
this.childModules.length = 0;
588+
this.errors = [];
589+
this.warnings = [];
582590

583-
// Remove this module from the initiators of old deps, so we can populate a
584-
// fresh cache
585-
this.dependencies.forEach(tModule => {
586-
tModule.initiators.delete(this);
587-
});
588-
this.transpilationDependencies.forEach(tModule => {
589-
tModule.transpilationInitiators.delete(this);
590-
});
591-
this.childModules.forEach(tModule => {
592-
tModule.dispose(manager);
591+
let code = this.module.code || '';
592+
let finalSourceMap = null;
593+
594+
const { requires } = this.module;
595+
if (requires != null && this.query === '') {
596+
// We now know that this has been transpiled on the server, so we shortcut
597+
const loaderContext = this.getLoaderContext(manager, {});
598+
// These are precomputed requires, for npm dependencies
599+
requires.forEach(r => {
600+
if (r.indexOf('glob:') === 0) {
601+
const reGlob = r.replace('glob:', '');
602+
loaderContext.addDependenciesInDirectory(reGlob);
603+
} else {
604+
loaderContext.addDependency(r);
605+
}
593606
});
594-
this.dependencies.clear();
595-
this.transpilationDependencies.clear();
596-
this.childModules.length = 0;
597-
this.errors = [];
598-
this.warnings = [];
599-
600-
let code = this.module.code || '';
601-
let finalSourceMap = null;
602-
603-
const { requires } = this.module;
604-
if (requires != null && this.query === '') {
605-
// We now know that this has been transpiled on the server, so we shortcut
606-
const loaderContext = this.getLoaderContext(manager, {});
607-
// These are precomputed requires, for npm dependencies
608-
requires.forEach(r => {
609-
if (r.indexOf('glob:') === 0) {
610-
const reGlob = r.replace('glob:', '');
611-
loaderContext.addDependenciesInDirectory(reGlob);
612-
} else {
613-
loaderContext.addDependency(r);
614-
}
615-
});
616607

617-
// eslint-disable-next-line
618-
code = this.module.code;
619-
} else {
620-
const transpilers = manager.preset.getLoaders(this.module, this.query);
621-
622-
const startTime = Date.now();
623-
for (let i = 0; i < transpilers.length; i += 1) {
624-
const transpilerConfig = transpilers[i];
625-
const loaderContext = this.getLoaderContext(
626-
manager,
627-
transpilerConfig.options || {}
628-
);
629-
loaderContext.remainingRequests = transpilers
630-
.slice(i + 1)
631-
.map(transpiler => transpiler.transpiler.name)
632-
.concat([this.module.path])
633-
.join('!');
608+
// eslint-disable-next-line
609+
code = this.module.code;
610+
} else {
611+
const transpilers = manager.preset.getLoaders(this.module, this.query);
612+
613+
const startTime = Date.now();
614+
for (let i = 0; i < transpilers.length; i += 1) {
615+
const transpilerConfig = transpilers[i];
616+
const loaderContext = this.getLoaderContext(
617+
manager,
618+
transpilerConfig.options || {}
619+
);
620+
loaderContext.remainingRequests = transpilers
621+
.slice(i + 1)
622+
.map(transpiler => transpiler.transpiler.name)
623+
.concat([this.module.path])
624+
.join('!');
625+
626+
try {
627+
const {
628+
transpiledCode,
629+
sourceMap,
630+
// eslint-disable-next-line no-await-in-loop
631+
} = await transpilerConfig.transpiler.transpile(code, loaderContext);
632+
633+
if (this.errors.length) {
634+
throw this.errors[0];
635+
}
634636

635-
try {
636-
const {
637-
transpiledCode,
638-
sourceMap,
639-
// eslint-disable-next-line no-await-in-loop
640-
} = await transpilerConfig.transpiler.transpile(
641-
code,
642-
loaderContext
643-
);
637+
code = transpiledCode;
638+
finalSourceMap = sourceMap;
639+
} catch (e) {
640+
e.fileName = loaderContext.path;
641+
e.tModule = this;
642+
this.resetTranspilation();
644643

645-
if (this.errors.length) {
646-
throw this.errors[0];
647-
}
644+
// Compilation should also be reset, since the code will be different now
645+
// we don't have a transpilation.
646+
this.resetCompilation();
647+
manager.clearCache();
648648

649-
code = transpiledCode;
650-
finalSourceMap = sourceMap;
651-
} catch (e) {
652-
e.fileName = loaderContext.path;
653-
e.tModule = this;
654-
this.resetTranspilation();
649+
throw e;
650+
}
651+
}
652+
debug(`Transpiled '${this.getId()}' in ${Date.now() - startTime}ms`);
655653

656-
// Compilation should also be reset, since the code will be different now
657-
// we don't have a transpilation.
658-
this.resetCompilation();
659-
manager.clearCache();
654+
this.logWarnings();
655+
}
660656

661-
throw e;
662-
}
663-
}
664-
debug(`Transpiled '${this.getId()}' in ${Date.now() - startTime}ms`);
657+
const sourceEqualsCompiled = code === this.module.code;
658+
const sourceURL = `//# sourceURL=${location.origin}${this.module.path}${
659+
this.query ? `?${this.hash}` : ''
660+
}`;
665661

666-
this.logWarnings();
667-
}
662+
// Add the source of the file by default, this is important for source mapping
663+
// errors back to their origin
664+
code = `${code}\n${sourceURL}`;
668665

669-
const sourceEqualsCompiled = code === this.module.code;
670-
const sourceURL = `//# sourceURL=${location.origin}${this.module.path}${
671-
this.query ? `?${this.hash}` : ''
672-
}`;
666+
this.source = new ModuleSource(
667+
this.module.path,
668+
code,
669+
finalSourceMap,
670+
sourceEqualsCompiled
671+
);
673672

674-
// Add the source of the file by default, this is important for source mapping
675-
// errors back to their origin
676-
code = `${code}\n${sourceURL}`;
673+
if (
674+
this.previousSource &&
675+
this.previousSource.compiledCode !== this.source.compiledCode
676+
) {
677+
const hasHMR = manager.preset
678+
.getLoaders(this.module, this.query)
679+
.some(t =>
680+
t.transpiler.HMREnabled == null ? true : t.transpiler.HMREnabled
681+
);
677682

678-
this.source = new ModuleSource(
679-
this.module.path,
680-
code,
681-
finalSourceMap,
682-
sourceEqualsCompiled
683-
);
683+
if (!hasHMR) {
684+
manager.markHardReload();
685+
} else {
686+
this.resetCompilation();
687+
}
688+
}
684689

685-
if (
686-
this.previousSource &&
687-
this.previousSource.compiledCode !== this.source.compiledCode
688-
) {
689-
const hasHMR = manager.preset
690-
.getLoaders(this.module, this.query)
691-
.some(t =>
692-
t.transpiler.HMREnabled == null ? true : t.transpiler.HMREnabled
693-
);
690+
await Promise.all(
691+
this.asyncDependencies.map(async p => {
692+
try {
693+
const tModule = await p;
694694

695-
if (!hasHMR) {
696-
manager.markHardReload();
697-
} else {
698-
this.resetCompilation();
695+
this.dependencies.add(tModule);
696+
tModule.initiators.add(this);
697+
} catch (e) {
698+
/* let this handle at evaluation */
699699
}
700-
}
700+
})
701+
);
701702

702-
await Promise.all(
703-
this.asyncDependencies.map(async p => {
704-
try {
705-
const tModule = await p;
703+
this.asyncDependencies = [];
706704

707-
this.dependencies.add(tModule);
708-
tModule.initiators.add(this);
709-
} catch (e) {
710-
/* let this handle at evaluation */
711-
}
712-
})
713-
);
705+
await Promise.all(
706+
flattenDeep([
707+
...Array.from(this.transpilationInitiators).map(t =>
708+
t.transpile(manager)
709+
),
710+
...Array.from(this.dependencies).map(t => t.transpile(manager)),
711+
])
712+
);
714713

715-
this.asyncDependencies = [];
714+
return this;
715+
}
716716

717-
await Promise.all(
718-
flattenDeep([
719-
...Array.from(this.transpilationInitiators).map(t =>
720-
t.transpile(manager)
721-
),
722-
...Array.from(this.dependencies).map(t => t.transpile(manager)),
723-
])
724-
);
717+
transpile(manager: Manager): Promise<this> {
718+
if (this.source) {
719+
return Promise.resolve(this);
720+
}
725721

726-
return this;
727-
} finally {
728-
delete manager.transpileJobs[this.getId()];
722+
const id = this.getId();
723+
if (manager.transpileJobs[id]) {
724+
if (manager.transpileJobs[id] === true) {
725+
// Is currently being transpiled, and the promise hasn't been set yet
726+
// because it is working on executing the promise. This rare case only
727+
// happens when we have a dependency loop, which could result in a
728+
// StackTraceOverflow. Dependency loop: A -> B -> C -> A -> B -> C
729+
return new Promise(async resolve => {
730+
while (!this.source && manager.transpileJobs[id] === true) {
731+
// eslint-disable-next-line
732+
await delay(10);
733+
}
734+
735+
if (manager.transpileJobs[id] && manager.transpileJobs[id] !== true) {
736+
resolve(manager.transpileJobs[id] as Promise<this>);
737+
} else {
738+
resolve(this);
739+
}
740+
});
741+
}
742+
return manager.transpileJobs[id] as Promise<this>;
729743
}
744+
745+
manager.transpileJobs[id] = true;
746+
// eslint-disable-next-line
747+
return (manager.transpileJobs[id] = this.doTranspile(manager)).finally(
748+
() => {
749+
delete manager.transpileJobs[id];
750+
}
751+
);
730752
}
731753

732754
logWarnings = () => {

packages/app/src/sandbox/startup.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ import BabelWorker from 'worker-loader?publicPath=/&name=babel-transpiler.[hash:
44
import hookConsole from 'sandbox-hooks/console';
55
import setupHistoryListeners from 'sandbox-hooks/url-listeners';
66
import { listenForPreviewSecret } from 'sandbox-hooks/preview-secret';
7+
import { isStandalone } from 'codesandbox-api';
78

89
window.babelworkers = [];
910
for (let i = 0; i < 3; i++) {
1011
window.babelworkers.push(new BabelWorker());
1112
}
1213

13-
if (!window.opener) {
14+
if (!window.opener && !isStandalone) {
1415
// Means we're in the editor
1516
setupHistoryListeners();
1617
hookConsole();

0 commit comments

Comments
 (0)