Skip to content
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 vite dep optimizer #1876

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 25 additions & 4 deletions packages/compat/src/compat-addons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { locateEmbroiderWorkingDir, RewrittenPackageCache, WaitForTrees } from '
import TreeSync from 'tree-sync';
import type CompatApp from './compat-app';
import { convertLegacyAddons } from './standalone-addon-build';
import { ensureSymlinkSync, writeFileSync, existsSync } from 'fs-extra';

// This build stage expects to be run with broccoli memoization enabled in order
// to get good rebuild performance. We turn it on by default here, but you can
Expand Down Expand Up @@ -47,11 +48,9 @@ export default class CompatAddons implements Stage {
},
changedMap: Map<string, boolean>
) {
let rewrittenPackages = resolve(locateEmbroiderWorkingDir(this.compatApp.root), 'rewritten-packages');
if (!this.treeSync) {
this.treeSync = new TreeSync(
addons,
resolve(locateEmbroiderWorkingDir(this.compatApp.root), 'rewritten-packages')
);
this.treeSync = new TreeSync(addons, rewrittenPackages);
}

if (
Expand All @@ -61,6 +60,28 @@ export default class CompatAddons implements Stage {
this.treeSync.sync();
RewrittenPackageCache.shared('embroider', this.compatApp.root).invalidateIndex();
}
const resolvableRewrittenPackages = resolve(
locateEmbroiderWorkingDir(this.compatApp.root),
'..',
'@embroider',
'rewritten-packages'
);
const embroiderDir = resolve(locateEmbroiderWorkingDir(this.compatApp.root), 'rewritten-packages');
console.log('link', embroiderDir, resolvableRewrittenPackages, existsSync(embroiderDir));
if (existsSync(embroiderDir)) {
ensureSymlinkSync(embroiderDir, resolvableRewrittenPackages, 'dir');
writeFileSync(
resolve(resolvableRewrittenPackages, 'package.json'),
JSON.stringify(
{
name: '@embroider/rewritten-packages',
main: 'moved-package-target.js',
},
null,
2
)
);
}
this.didBuild = true;
}
}
142 changes: 130 additions & 12 deletions packages/core/src/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
packageName as getPackageName,
packageName,
} from '@embroider/shared-internals';
import { dirname, resolve, posix } from 'path';
import { dirname, resolve, posix, extname } from 'path';
import type { Package, V2Package } from '@embroider/shared-internals';
import { explicitRelative, RewrittenPackageCache } from '@embroider/shared-internals';
import makeDebug from 'debug';
Expand Down Expand Up @@ -82,6 +82,7 @@ function isTerminal(request: ModuleRequest): boolean {
}

export interface Options {
makeAbsolutePathToRwPackages?: string[];
renamePackages: {
[fromName: string]: string;
};
Expand Down Expand Up @@ -232,8 +233,8 @@ export class Resolver {

switch (resolution.type) {
case 'found':
case 'ignored':
return resolution;
case 'ignored':
case 'not_found':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignored should fallback resolve

break;
default:
Expand Down Expand Up @@ -411,6 +412,7 @@ export class Resolver {
}

let pkg = this.packageCache.ownerOfFile(request.fromFile);
pkg = pkg && this.packageCache.maybeMoved(pkg);
if (!pkg?.isV2Ember()) {
throw new Error(`bug: found implicit modules import in non-ember package at ${request.fromFile}`);
}
Expand Down Expand Up @@ -638,6 +640,7 @@ export class Resolver {

let hbsModule: Resolution | null = null;
let jsModule: Resolution | null = null;
let jsSpecifier: string | null = null;

// first, the various places our template might be.
for (let candidate of this.componentTemplateCandidates(target.packageName)) {
Expand Down Expand Up @@ -669,6 +672,7 @@ export class Resolver {
// It matches as a priority lower than .js, so finding an .hbs means
// there's definitely not a .js.
if (resolution.type === 'found' && !resolution.filename.endsWith('.hbs')) {
jsSpecifier = candidateSpecifier;
jsModule = resolution;
break;
}
Expand All @@ -680,8 +684,20 @@ export class Resolver {
request,
request.virtualize(virtualPairComponent(hbsModule.filename, jsModule?.filename))
);
} else if (jsModule) {
return logTransition(`resolving to resolveComponent found only JS`, request, request.resolveTo(jsModule));
} else if (jsSpecifier && jsModule) {
let newRequest = request.alias(jsSpecifier);
let fromPkg = this.packageCache.ownerOfFile(target.from)!;
let renamedRequest = this.handleRenaming(newRequest);
if (!renamedRequest.specifier.startsWith('.')) {
newRequest = renamedRequest;
}
let targetPkgName = getPackageName(newRequest.specifier);
if (targetPkgName && fromPkg.name !== targetPkgName) {
newRequest = this.makeResolvable(newRequest);
} else {
newRequest = renamedRequest;
}
return logTransition(`resolving to resolveComponent found only JS`, request, newRequest);
} else {
return logTransition(`resolveComponent failed`, request);
}
Expand All @@ -703,7 +719,11 @@ export class Resolver {
);

if (helperMatch.type === 'found') {
return logTransition('resolve to ambiguous case matched a helper', request, request.resolveTo(helperMatch));
return logTransition(
'resolve to ambiguous case matched a helper',
request,
request.alias(helperCandidate.specifier)
);
}

// unlike resolveHelper, resolveComponent already does pre-resolution in
Expand Down Expand Up @@ -899,6 +919,9 @@ export class Resolver {
if (isTerminal(request)) {
return request;
}
if (request.specifier.startsWith('@embroider/rewritten-packages')) {
return request;
}
let requestingPkg = this.packageCache.ownerOfFile(request.fromFile);
if (!requestingPkg) {
return request;
Expand Down Expand Up @@ -1061,12 +1084,20 @@ export class Resolver {
}

private resolveWithinMovedPackage<R extends ModuleRequest>(request: R, pkg: Package): R {
let levels = ['..'];
if (pkg.name.startsWith('@')) {
levels.push('..');
}
let originalFromFile = request.fromFile;
let newRequest = request.rehome(resolve(pkg.root, ...levels, 'moved-package-target.js'));
if (!pkg.isV2Addon()) {
let levels = ['..'];
if (pkg.name.startsWith('@')) {
levels.push('..');
}
let newRequest = request.rehome(resolve(pkg.root, ...levels, 'moved-package-target.js'));

// setting meta because if this fails, we want the fallback to pick up back
// in the original requesting package.
return newRequest.withMeta({ originalFromFile });
}

let newRequest = this.makeResolvable(request);

if (newRequest === request) {
return request;
Expand Down Expand Up @@ -1247,13 +1278,97 @@ export class Resolver {
}
}

async resolveAlias<R extends ModuleRequest>(request: R): Promise<R> {
let engineNames = this.options.engines.map(e => e.packageName);
let res = {
importer: request.fromFile,
path: request.specifier,
};
let isVirtual = request.isVirtual;
let path = request.specifier;
if (path.startsWith('.') || path.startsWith('#') || engineNames.some(packageName => path.startsWith(packageName))) {
let resolved = await this.beforeResolve(request);
if (resolved.isVirtual) {
isVirtual = true;
let result = await this.resolve(request);
if (result.type !== 'not_found') {
const r = result.result as any;
if (r?.path) {
res.path = r.path;
res.importer = r.importer;
} else if (r?.specifier) {
res.path = r.specifier;
res.importer = r.fromFile;
}
}
}
if (!resolved.isVirtual) {
resolved = await this.fallbackResolve(resolved);
if (resolved.specifier) {
res.path = resolved.specifier;
res.importer = resolved.fromFile;
}
}
} else {
let resolved = this.handleRenaming(request);
if (resolved.specifier) {
res.path = resolved.specifier;
res.importer = resolved.fromFile;
}
}
if (!isVirtual) {
const extNameRegex = new RegExp(`\\${extname(res.path)}$`);
res.path = res.path.replace(extNameRegex, '');
}
return request.alias(res.path).rehome(res.importer);
}

makeResolvable<R extends ModuleRequest>(request: R): R {
if (request.fromFile && !request.fromFile.startsWith('./')) {
let fromPkg: Package;
try {
fromPkg =
this.packageCache.ownerOfFile(request.fromFile) || this.packageCache.ownerOfFile(this.options.appRoot)!;
} catch (e) {
console.log(e);
fromPkg = this.packageCache.ownerOfFile(this.options.appRoot)!;
}

let pkgName = getPackageName(request.specifier);
try {
let pkg = pkgName ? this.packageCache.resolve(pkgName, fromPkg!) : fromPkg;
if (!pkg.isV2Addon() || !pkg.meta['auto-upgraded'] || !pkg.root.includes('rewritten-packages')) {
// some tests make addons be auto-upgraded, but are not actually in rewritten-packages
return request;
}
let levels = ['..'];
if (pkg.name.startsWith('@')) {
levels.push('..');
}
let resolvedRoot = resolve(pkg.root, ...levels, ...levels, '..');
let specifier = resolve(pkg.root, ...levels, request.specifier);
let makeAbsolutePath = this.options.makeAbsolutePathToRwPackages;
if (makeAbsolutePath && makeAbsolutePath.some((addon: string) => request.specifier.startsWith(addon))) {
return request.alias(specifier);
}
specifier = specifier.replace(resolvedRoot, '@embroider/rewritten-packages').replace(/\\/g, '/');
return request.alias(specifier).rehome(resolve(this.options.appRoot, 'package.json'));
} catch (e) {}
}
return request;
}

private async fallbackResolve<R extends ModuleRequest>(request: R): Promise<R> {
if (request.isVirtual) {
throw new Error(
'Build tool bug detected! Fallback resolve should never see a virtual request. It is expected that the defaultResolve for your bundler has already resolved this request'
);
}

if (request.specifier.startsWith('@embroider/rewritten-packages')) {
return request;
}

if (request.specifier === '@embroider/macros') {
// the macros package is always handled directly within babel (not
// necessarily as a real resolvable package), so we should not mess with it.
Expand Down Expand Up @@ -1288,9 +1403,10 @@ export class Resolver {
// but then come back to the original location here in the fallback when the
// rehomed request fails
let movedPkg = this.packageCache.maybeMoved(pkg);
if (movedPkg !== pkg) {
if (movedPkg !== pkg && !movedPkg.isV2App()) {
let originalFromFile = request.meta?.originalFromFile;
if (typeof originalFromFile !== 'string') {
console.log(pkg, movedPkg, request.specifier);
throw new Error(`bug: embroider resolver's meta is not propagating`);
}
request = request.rehome(originalFromFile);
Expand All @@ -1315,6 +1431,7 @@ export class Resolver {
explicitRelative(pkg.root, resolve(dirname(request.fromFile), request.specifier))
);
if (appJSMatch) {
appJSMatch = this.makeResolvable(appJSMatch);
return logTransition('fallbackResolve: relative appJsMatch', request, appJSMatch);
} else {
return logTransition('fallbackResolve: relative appJs search failure', request);
Expand All @@ -1331,7 +1448,8 @@ export class Resolver {
if (addon) {
const rehomed = request.rehome(addon.canResolveFromFile);
if (rehomed !== request) {
return logTransition(`activeAddons`, request, rehomed);
let newRequest = this.makeResolvable(rehomed);
return logTransition(`activeAddons`, request, newRequest);
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/resolver-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,23 @@ import { join } from 'path';
import type { FSWatcher } from 'fs';
import { watch as fsWatch } from 'fs';

const instances = new Map();

export class ResolverLoader {
#resolver: Resolver | undefined;
#configFile: string;
#watcher: FSWatcher | undefined;
sharedConfig: any;

constructor(readonly appRoot: string, watch = false) {
this.#configFile = join(locateEmbroiderWorkingDir(this.appRoot), 'resolver.json');
this.sharedConfig = {};

if (instances.has(appRoot)) {
return instances.get(appRoot);
}
instances.set(appRoot, this);

if (watch) {
this.#watcher = fsWatch(this.#configFile, { persistent: false }, () => {
this.#resolver = undefined;
Expand All @@ -28,6 +38,7 @@ export class ResolverLoader {
if (!this.#resolver) {
let config: Options = readJSONSync(join(locateEmbroiderWorkingDir(this.appRoot), 'resolver.json'));
this.#resolver = new Resolver(config);
this.#resolver.options.makeAbsolutePathToRwPackages = this.sharedConfig.excludeLegacyAddons;
}
return this.#resolver;
}
Expand Down
8 changes: 6 additions & 2 deletions packages/shared-internals/src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ export default class Package {

@Memoize()
protected get internalPackageJSON() {
return JSON.parse(readFileSync(join(this.root, 'package.json'), 'utf8'));
try {
return JSON.parse(readFileSync(join(this.root, 'package.json'), 'utf8'));
Copy link
Contributor Author

@patricklx patricklx Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workaround for fromFile pointing at /app/tests/package.json and vite/deps

} catch {
return {};
}
}

@Memoize()
Expand All @@ -43,7 +47,7 @@ export default class Package {
}

get meta(): AddonMeta | AppMeta | undefined {
let m = this.packageJSON['ember-addon'];
let m = this.packageJSON['ember-addon'] || {};
if (this.isV2App()) {
return m as unknown as AppMeta;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function compatPrebuild(): Plugin {
viteCommand = command;
viteMode = mode;
},
async buildStart() {
async configResolved() {
if (!viteCommand) {
throw new Error(`bug: embroider compatPrebuild did not detect Vite's command`);
}
Expand Down
14 changes: 8 additions & 6 deletions packages/vite/src/esbuild-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,20 @@ export class EsBuildModuleRequest implements ModuleRequest {
isVirtual: this.isVirtual,
};
}

if (request.isNotFound) {
// todo: make sure this looks correct to users
return {
type: 'not_found',
err: {
errors: [{ text: `module not found ${request.specifier}` }],
type: 'ignored',
result: {
external: true,
namespace: '',
},
};
}

let from = this.fromFile;
let result = await this.context.resolve(request.specifier, {
importer: request.fromFile,
importer: from,
resolveDir: dirname(request.fromFile),
kind: this.kind,
pluginData: {
Expand All @@ -156,7 +158,7 @@ export class EsBuildModuleRequest implements ModuleRequest {
},
});
if (result.errors.length > 0) {
return { type: 'not_found', err: result };
return { type: 'ignored', result };
} else if (result.external) {
return { type: 'ignored', result };
} else {
Expand Down