Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
@mischnic
mischnic committed Oct 10, 2023
1 parent 1a3a254 commit a8345f7
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 35 deletions.
156 changes: 121 additions & 35 deletions packages/core/core/src/BundleGraph.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {getBundleGroupId, getPublicId} from './utils';
import {ISOLATED_ENVS} from './public/Environment';
import {fromProjectPath, fromProjectPathRelative} from './projectPath';
import {HASH_REF_PREFIX} from './constants';
import inspect from 'graphql/jsutils/inspect';

export const bundleGraphEdgeTypes = {
// A lack of an edge type indicates to follow the edge while traversing
Expand Down Expand Up @@ -205,6 +206,13 @@ export default class BundleGraph {
// Disable in dev mode because this feature is at odds with safeToIncrementallyBundle
isProduction
) {
let nodeValueSymbols = node.value.symbols;

let source = nullthrows(
graph.getNodeByContentKey(nullthrows(node.value.sourceAssetId)),

This comment has been minimized.

Sign in to view

Copy link
@thebriando

thebriando Oct 16, 2023

Contributor

I'm running into an issue where it's hitting the nullthrows here because some of the source nodes aren't in the graph. I had to change a couple things to make it work. Any thoughts? 8ac0ab0
);
invariant(source.type === 'asset');

// asset -> symbols that should be imported directly from that asset
let targets = new DefaultMap<ContentKey, Map<Symbol, Symbol>>(
() => new Map(),
Expand Down Expand Up @@ -252,36 +260,88 @@ export default class BundleGraph {
([, t]) => new Set([...t.values()]).size === t.size,
)
) {
console.log(
node.value.specifier,
[...targets].map(([a, s]) => [
assetGraph.getNodeByContentKey(a)?.value.filePath,
s,
]),
);

let sourceAssetSymbols = nullthrows(source.value.symbols);
let isReexportAll = nodeValueSymbols.get('*')?.local === '*';
let reexportAllLoc = isReexportAll
? nullthrows(nodeValueSymbols.get('*')).loc
: undefined;

// TODO adjust sourceAssetIdNode.value.dependencies ?
let deps = [
// Keep the original dependency
{
asset: null,
dep: graph.addNodeByContentKey(node.id, {
...node,
value: {
...node.value,
symbols: node.value.symbols
? new Map(
[...node.value.symbols].filter(([k]) =>
externalSymbols.has(k),
),
)
: undefined,
},
usedSymbolsUp: new Map(
[...node.usedSymbolsUp].filter(([k]) =>
externalSymbols.has(k),
),
),
usedSymbolsDown: new Set(),
excluded: externalSymbols.size === 0,
}),
},
// {

This comment has been minimized.

Sign in to view

Copy link
@thebriando

thebriando Oct 16, 2023

Contributor

Why did this get removed?
// asset: null,
// dep: graph.addNodeByContentKey(node.id, {
// ...node,
// value: {
// ...node.value,
// symbols: new Map(
// [...nodeValueSymbols].filter(([k]) =>
// externalSymbols.has(k),
// ),
// ),
// },
// usedSymbolsUp: new Map(
// [...node.usedSymbolsUp].filter(([k]) =>
// externalSymbols.has(k),
// ),
// ),
// usedSymbolsDown: new Set(),
// excluded: externalSymbols.size === 0,
// }),
// },
...[...targets].map(([asset, target]) => {
let newNodeId = hashString(
node.id + [...target.keys()].join(','),
);

let symbols = new Map();
console.log(
'X',
require('util').inspect(nodeValueSymbols, {depth: Infinity}),
target,
isReexportAll,
);
for (let [as, from] of target) {
let existing = nodeValueSymbols.get(as);
if (existing) {
symbols.set(from, existing);
} else {
invariant(isReexportAll);
let local = `${node.value.id}$rewrite$${asset}$${from}`;
symbols.set(from, {
isWeak: true,
local,
loc: reexportAllLoc,
});
console.log(
'adding ',
as,
source.value.filePath,
sourceAssetSymbols,
);
// It might already exist with multiple export-alls causing ambiguous resolution
if (!sourceAssetSymbols.has(as)) {
sourceAssetSymbols.set(as, {
loc: reexportAllLoc,
local: local,
});
}
}
}
let usedSymbolsUp = new Map(
[...node.usedSymbolsUp]
.filter(([k]) => target.has(k) || k === '*')
.map(([k, v]) => [target.get(k) ?? k, v]),
);
return {
asset,
dep: graph.addNodeByContentKey(newNodeId, {
Expand All @@ -290,24 +350,28 @@ export default class BundleGraph {
value: {
...node.value,
id: newNodeId,
symbols: node.value.symbols
? new Map(
[...node.value.symbols]
.filter(([k]) => target.has(k) || k === '*')
.map(([k, v]) => [target.get(k) ?? k, v]),
)
: undefined,
symbols,
},
usedSymbolsUp: new Map(
[...node.usedSymbolsUp]
.filter(([k]) => target.has(k) || k === '*')
.map(([k, v]) => [target.get(k) ?? k, v]),
),
usedSymbolsUp,
// This is only a temporary helper needed during symbol propagation and is never
// read afterwards.
usedSymbolsDown: new Set(),
}),
};
}),
];
console.log(
node.value.specifier,
require('util').inspect(
deps.map(x => [
assetGraph.getNodeByContentKey(x.asset)?.value.filePath,
[...graph.getNode(x.dep).usedSymbolsUp],
[...graph.getNode(x.dep).value.symbols],
]),
{depth: Infinity},
),
);

dependencies.set(nodeId, deps);

// Jump to the dependencies that are used in this dependency
Expand Down Expand Up @@ -1643,6 +1707,15 @@ export default class BundleGraph {
symbol: Symbol,
boundary: ?Bundle,
): InternalSymbolResolution {
// let isX = String(asset.filePath) === 'lib.js' && symbol === 'Foo';
// let isY = String(asset.filePath) === 'libFoo2.js' && symbol === 'Foo';
// if (isX) {
// global.X = true;
// }
// try {
if (global.X) {
console.log('enter', asset.filePath, symbol);
}
let assetOutside = boundary && !this.bundleHasAsset(boundary, asset);

let identifier = asset.symbols?.get(symbol)?.local;
Expand Down Expand Up @@ -1727,6 +1800,9 @@ export default class BundleGraph {
continue;
}
let result = this.getSymbolResolution(resolved, symbol, boundary);
// if (global.X) {
// console.log('recursive', resolved.filePath, symbol, result);
// }

// We found the symbol
if (result.symbol != undefined) {
Expand Down Expand Up @@ -1777,6 +1853,11 @@ export default class BundleGraph {
}
}
}

// if (global.X) {
// console.log(asset.filePath, symbol, potentialResults);
// }

// We didn't find the exact symbol...
if (potentialResults.length == 1) {
// ..., but if it does exist, it has to be behind this one reexport.
Expand Down Expand Up @@ -1810,6 +1891,11 @@ export default class BundleGraph {
loc: asset.symbols?.get(symbol)?.loc,
};
}
// } finally {
// if (isX) {
// global.X = false;
// }
// }
}
getAssetById(contentKey: string): Asset {
let node = this._graph.getNodeByContentKey(contentKey);
Expand Down
4 changes: 4 additions & 0 deletions packages/packagers/js/src/ScopeHoistingPackager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,10 @@ ${code}
symbol,
} = this.bundleGraph.getSymbolResolution(resolved, imported, this.bundle);

// if (imported === 'Foo') {
// console.log({resolved, imported}, {resolvedAsset, exportSymbol, symbol});
// }

if (
resolvedAsset.type !== 'js' ||
(dep && this.bundleGraph.isDependencySkipped(dep))
Expand Down
1 change: 1 addition & 0 deletions packages/transformers/js/src/JSTransformer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,7 @@ export default (new Transformer({
}

asset.type = 'js';
// console.log(asset.filePath, compiledCode.toString())
asset.setBuffer(compiledCode);

if (map) {
Expand Down

0 comments on commit a8345f7

Please sign in to comment.