Skip to content

Commit e36a604

Browse files
NullVoxPopuliclaude
andcommitted
[BUGFIX] Preserve barrel side effects when rewriting deep imports
The no-barrel-imports rule rewrites `import { x } from 'BARREL'` to deep paths like `import { x } from 'BARREL/lib/foo'` for tree-shaking. But when the barrel itself has top-level side effects (e.g. `import './lib/bootstrap';` in @glimmer/runtime/index.ts, or the `globalThis` registration check in @glimmer/validator/index.ts), those side effects are skipped — the barrel is never loaded. Concrete failure: @glimmer/runtime/index.ts pulls in bootstrap.ts as a side-effect import. bootstrap.ts then imports ./compiled/opcodes/{component,content,dom,debugger,expressions,vm,lists}, each of which calls APPEND_OPCODES.add(...) at module-load to register opcode handlers. Without bootstrap, AppendOpcodes.evaluateOpcode[type] is null at render time → "Cannot read properties of null (reading 'syscall')" in the smoke tests. The rule now: 1. Detects whether a barrel has top-level side effects (any non-export, non-pure-type, non-`declare` statement, or any side-effect-only import). 2. When rewriting away from such a barrel, prepends a side-effect-only `import 'BARREL';` to the rewritten output. Modules are cached, so this is idempotent; bundlers will tree-shake the named exports and leave only the side effects, which is exactly what we want. 3. Skips this for `import type` / `export type` (no runtime). Side-effect detection caught and preserved three barrels we'd otherwise have silently broken: @glimmer/runtime (opcode bootstrap), @glimmer/validator (duplicate-package guard), and @ember/object (EmberObject extends CoreObject.extend(Observable) at top level). The Fix commit's manual one-off patches in @glimmer/debug and @glimmer/opcode-compiler aren't needed anymore — the rule now handles the IS_COMPILABLE_TEMPLATE type/value distinction correctly via its existing isType tracking. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 753eba9 commit e36a604

70 files changed

Lines changed: 154 additions & 6 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

eslint-rules/no-barrel-imports.js

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const WORKSPACE_ROOT = path.resolve(__dirname, '..');
99
const PACKAGES_ROOT = path.join(WORKSPACE_ROOT, 'packages');
1010

1111
const moduleCache = new Map();
12+
const sideEffectsCache = new Map();
1213
const owningPackageCache = new Map();
1314
const wildcardExportsCache = new Map();
1415

@@ -87,11 +88,13 @@ function getModuleExports(filepath, stack = new Set()) {
8788
ast = parse(fs.readFileSync(filepath, 'utf8'), { jsx: false, range: true, loc: true });
8889
} catch {
8990
moduleCache.set(filepath, null);
91+
sideEffectsCache.set(filepath, false);
9092
stack.delete(filepath);
9193
return null;
9294
}
9395

9496
const exports = new Map();
97+
let hasSideEffects = false;
9598
for (const stmt of ast.body) {
9699
if (stmt.type === 'ExportNamedDeclaration') collectNamedExports(stmt, exports, filepath, stack);
97100
else if (stmt.type === 'ExportAllDeclaration')
@@ -103,14 +106,41 @@ function getModuleExports(filepath, stack = new Set()) {
103106
isType: false,
104107
kind: 'local',
105108
});
109+
} else if (
110+
stmt.type === 'ImportDeclaration' &&
111+
(stmt.specifiers?.length ?? 0) === 0 &&
112+
stmt.importKind !== 'type'
113+
) {
114+
// Bare side-effect-only import: `import 'foo';`
115+
hasSideEffects = true;
116+
} else if (
117+
stmt.type !== 'ImportDeclaration' &&
118+
stmt.type !== 'TSImportEqualsDeclaration' &&
119+
stmt.type !== 'TSModuleDeclaration' &&
120+
stmt.type !== 'TSInterfaceDeclaration' &&
121+
stmt.type !== 'TSTypeAliasDeclaration' &&
122+
stmt.type !== 'TSDeclareFunction' &&
123+
// `declare` statements (`declare const x: ...`, `declare function f()`)
124+
// and ambient enums are TS-only — they emit nothing at runtime.
125+
!stmt.declare
126+
) {
127+
// Any other top-level statement that isn't an import or a pure type-level
128+
// declaration is potentially side-effecting (e.g. `someFn()` or `let x = 1`).
129+
hasSideEffects = true;
106130
}
107131
}
108132

109133
moduleCache.set(filepath, exports);
134+
sideEffectsCache.set(filepath, hasSideEffects);
110135
stack.delete(filepath);
111136
return exports;
112137
}
113138

139+
function moduleHasSideEffects(filepath) {
140+
if (!sideEffectsCache.has(filepath)) getModuleExports(filepath);
141+
return sideEffectsCache.get(filepath) === true;
142+
}
143+
114144
function collectNamedExports(stmt, exports, filepath, stack) {
115145
const stmtIsType = stmt.exportKind === 'type';
116146

@@ -402,6 +432,23 @@ module.exports = {
402432
return missing;
403433
}
404434

435+
// If `barrelPath` has top-level side effects (e.g. a side-effect-only
436+
// `import './lib/bootstrap';`), bypassing the barrel skips those side
437+
// effects. Returns a side-effect-only import statement that re-establishes
438+
// them when prepended to the rewritten output, or `null` if the barrel has
439+
// no side effects.
440+
function sideEffectImportFor(barrelPath, barrelSpec) {
441+
if (!moduleHasSideEffects(barrelPath)) return null;
442+
const currentInfo = getPackageInfo(filename);
443+
const targetInfo = getPackageInfo(barrelPath);
444+
if (currentInfo && targetInfo && currentInfo.bareSpec === targetInfo.bareSpec) {
445+
let rel = toPosix(path.relative(path.dirname(filename), barrelPath));
446+
if (!rel.startsWith('.')) rel = './' + rel;
447+
return `import '${stripExt(rel)}';`;
448+
}
449+
return `import '${barrelSpec}';`;
450+
}
451+
405452
function check(node) {
406453
const spec = node.source?.value;
407454
if (typeof spec !== 'string') return;
@@ -492,10 +539,21 @@ module.exports = {
492539
// be sourced from a sub-file because they don't live in one.
493540
for (const item of kept) pushGroup(groups, spec, item);
494541

495-
const statements = namespaceImports.map((ns) => {
542+
const statements = [];
543+
// Type-only imports (`import type { ... } from 'foo';`) don't trigger
544+
// module evaluation at runtime, so there are no side effects to preserve.
545+
// For everything else, if the barrel has top-level side effects,
546+
// bypassing it would skip them — emit a bare side-effect import.
547+
if (!isWholeTypeImport) {
548+
const sideEffectImport = sideEffectImportFor(barrelPath, spec);
549+
if (sideEffectImport) statements.push(sideEffectImport);
550+
}
551+
for (const ns of namespaceImports) {
496552
const useType = isWholeTypeImport || ns.isType;
497-
return `import ${useType ? 'type ' : ''}* as ${ns.localName} from '${ns.newSpec}';`;
498-
});
553+
statements.push(
554+
`import ${useType ? 'type ' : ''}* as ${ns.localName} from '${ns.newSpec}';`
555+
);
556+
}
499557
if (groups.size > 0) statements.push(buildImportStatements(groups, isWholeTypeImport));
500558

501559
const allSpecs = [...namespaceImports.map((n) => n.newSpec), ...groups.keys()];
@@ -564,10 +622,17 @@ module.exports = {
564622

565623
for (const item of kept) pushGroup(groups, spec, item);
566624

567-
const statements = namespaceExports.map((ns) => {
625+
const statements = [];
626+
if (!isWholeTypeExport) {
627+
const sideEffectImport = sideEffectImportFor(barrelPath, spec);
628+
if (sideEffectImport) statements.push(sideEffectImport);
629+
}
630+
for (const ns of namespaceExports) {
568631
const useType = isWholeTypeExport || ns.isType;
569-
return `export ${useType ? 'type ' : ''}* as ${ns.exportedName} from '${ns.newSpec}';`;
570-
});
632+
statements.push(
633+
`export ${useType ? 'type ' : ''}* as ${ns.exportedName} from '${ns.newSpec}';`
634+
);
635+
}
571636
if (groups.size > 0) statements.push(buildExportStatements(groups, isWholeTypeExport));
572637

573638
const allSpecs = [...namespaceExports.map((n) => n.newSpec), ...groups.keys()];

packages/@ember/-internals/glimmer/lib/component-managers/curly.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ import {
3636
createPrimitiveRef,
3737
valueForRef,
3838
} from '@glimmer/reference/lib/reference';
39+
import '@glimmer/runtime';
3940
import { reifyPositional } from '@glimmer/runtime/lib/vm/arguments';
4041
import { EMPTY_ARRAY } from '@glimmer/util/lib/array-utils';
4142
import { unwrapTemplate } from './unwrap-template';
43+
import '@glimmer/validator';
4244
import {
4345
beginTrackFrame,
4446
beginUntrackFrame,

packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {
1818
import { capabilityFlagsFrom } from '@glimmer/manager/lib/util/capabilities';
1919
import type { Reference } from '@glimmer/reference/lib/reference';
2020
import { UNDEFINED_REFERENCE, valueForRef } from '@glimmer/reference/lib/reference';
21+
import '@glimmer/runtime';
2122
import { EMPTY_ARGS } from '@glimmer/runtime/lib/vm/arguments';
2223
import { unwrapTemplate } from './unwrap-template';
2324

packages/@ember/-internals/glimmer/lib/component-managers/root.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
} from '@glimmer/interfaces';
1212
import type { Nullable } from '@ember/-internals/utility-types';
1313
import { capabilityFlagsFrom } from '@glimmer/manager/lib/util/capabilities';
14+
import '@glimmer/validator';
1415
import { CONSTANT_TAG } from '@glimmer/validator/lib/validators';
1516
import { consumeTag } from '@glimmer/validator/lib/tracking';
1617
import type Component from '../component';

packages/@ember/-internals/glimmer/lib/component-managers/route-template.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { DEBUG } from '@glimmer/env';
1818
import { capabilityFlagsFrom } from '@glimmer/manager/lib/util/capabilities';
1919
import type { Reference } from '@glimmer/reference/lib/reference';
2020
import { createDebugAliasRef, valueForRef } from '@glimmer/reference/lib/reference';
21+
import '@glimmer/runtime';
2122
import { curry, type CurriedValue } from '@glimmer/runtime/lib/curried-value';
2223
import { unwrapTemplate } from './unwrap-template';
2324

packages/@ember/-internals/glimmer/lib/component.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ import { DEBUG } from '@glimmer/env';
2020
import type { Environment, Template, TemplateFactory } from '@glimmer/interfaces';
2121
import { setInternalComponentManager } from '@glimmer/manager/lib/internal/api';
2222
import { isUpdatableRef, updateRef } from '@glimmer/reference/lib/reference';
23+
import '@glimmer/runtime';
2324
import { normalizeProperty } from '@glimmer/runtime/lib/dom/props';
2425
import type { DirtyableTag } from '@glimmer/interfaces';
26+
import '@glimmer/validator';
2527
import { createTag, DIRTY_TAG as dirtyTag } from '@glimmer/validator/lib/validators';
2628
import type { SimpleElement } from '@simple-dom/interface';
2729
import {

packages/@ember/-internals/glimmer/lib/components/input.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { type Opaque } from '@ember/-internals/utility-types';
66
import { assert, warn } from '@ember/debug';
77
import { action } from '@ember/object';
88
import { valueForRef } from '@glimmer/reference/lib/reference';
9+
import '@glimmer/validator';
910
import { untrack } from '@glimmer/validator/lib/tracking';
1011
import InputTemplate from '../templates/input';
1112
import AbstractInput, { valueFrom } from './abstract-input';

packages/@ember/-internals/glimmer/lib/components/internal.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { setComponentTemplate } from '@glimmer/manager/lib/public/template';
1717
import { setInternalComponentManager } from '@glimmer/manager/lib/internal/api';
1818
import type { Reference } from '@glimmer/reference/lib/reference';
1919
import { createConstRef, isConstRef, valueForRef } from '@glimmer/reference/lib/reference';
20+
import '@glimmer/validator';
2021
import { untrack } from '@glimmer/validator/lib/tracking';
2122

2223
function NOOP(): void {}

packages/@ember/-internals/glimmer/lib/components/link-to.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type Route from '@ember/routing/route';
22
import type { RouterState, RoutingService } from '@ember/routing/-internals';
33
import { isSimpleClick } from '@ember/-internals/views/lib/system/utils';
4+
import '@ember/debug';
45
import inspect from '@ember/debug/lib/inspect';
56
import { assert, debugFreeze, warn } from '@ember/debug';
67
import { getEngineParent } from '@ember/engine/parent';
@@ -11,6 +12,7 @@ import { service } from '@ember/service';
1112
import { DEBUG } from '@glimmer/env';
1213
import type { Maybe } from '@glimmer/interfaces';
1314
import type { Nullable } from '@ember/-internals/utility-types';
15+
import '@glimmer/validator';
1416
import { consumeTag, createCache, getValue, untrack } from '@glimmer/validator/lib/tracking';
1517
import { tagFor } from '@glimmer/validator/lib/meta';
1618
import type { Transition } from 'router_js';

packages/@ember/-internals/glimmer/lib/dom.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import '@glimmer/runtime';
12
export { DOMChanges } from '@glimmer/runtime/lib/dom/helper';
23
export { DOMTreeConstruction } from '@glimmer/runtime/lib/dom/api';
34
export { clientBuilder } from '@glimmer/runtime/lib/vm/element-builder';

0 commit comments

Comments
 (0)