Skip to content

Commit

Permalink
Merge pull request #1070 from NullVoxPopuli/use-stricter-dependency-r…
Browse files Browse the repository at this point in the history
…esolution-in-dep-satisfies

Do not try to resolve packages that are not declared in dependencies
  • Loading branch information
ef4 authored Jan 12, 2022
2 parents f7a4ad0 + 9b9fd6d commit f39b6f0
Show file tree
Hide file tree
Showing 45 changed files with 475 additions and 403 deletions.
8 changes: 4 additions & 4 deletions packages/compat/src/compat-addons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default class CompatAddons implements Stage {
ensureDirSync(options.workspaceDir!);
this.destDir = realpathSync(options.workspaceDir!);

this.packageCache = v1Cache.packageCache.moveAddons(v1Cache.app.root, this.destDir);
this.packageCache = v1Cache.app.packageCache.moveAddons(this.destDir);
this.inputPath = v1Cache.app.root;
this.treeSyncMap = new WeakMap();
this.v1Cache = v1Cache;
Expand Down Expand Up @@ -57,16 +57,16 @@ export default class CompatAddons implements Stage {
async ready(): Promise<{ outputPath: string; packageCache: PackageCache }> {
await this.deferReady.promise;
writeJSONSync(join(this.destDir, '.embroider-reuse.json'), {
appDestDir: relative(this.destDir, this.packageCache.appDestDir),
appDestDir: relative(this.destDir, this.packageCache.appRoot),
});
return {
outputPath: this.packageCache.appDestDir,
outputPath: this.packageCache.appRoot,
packageCache: this.packageCache,
};
}

private get appDestDir(): string {
return this.packageCache.appDestDir;
return this.packageCache.appRoot;
}

private get app(): Package {
Expand Down
11 changes: 9 additions & 2 deletions packages/compat/src/compat-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function setup(legacyEmberAppInstance: object, options: Required<Options>) {
};

let instantiate = async (root: string, appSrcDir: string, packageCache: PackageCache) => {
let appPackage = packageCache.getApp(appSrcDir);
let appPackage = packageCache.get(appSrcDir);
let adapter = new CompatAppAdapter(
root,
appPackage,
Expand All @@ -82,7 +82,13 @@ function setup(legacyEmberAppInstance: object, options: Required<Options>) {
packageCache.get(join(root, 'node_modules', '@embroider', 'synthesized-styles'))
);

return new AppBuilder<TreeNames>(root, appPackage, adapter, options, MacrosConfig.for(legacyEmberAppInstance));
return new AppBuilder<TreeNames>(
root,
appPackage,
adapter,
options,
MacrosConfig.for(legacyEmberAppInstance, appSrcDir)
);
};

return { inTrees, instantiate };
Expand Down Expand Up @@ -387,6 +393,7 @@ class CompatAppAdapter implements AppAdapter<TreeNames> {
// persistent caching.
externalsDir: join(tmpdir, 'embroider', 'externals'),
emberNeedsModulesPolyfill,
appRoot: this.root,
};
}

Expand Down
20 changes: 11 additions & 9 deletions packages/compat/src/moved-package-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ function assertNoTildeExpansion(source: string, target: string) {
}
}
export class MovablePackageCache extends PackageCache {
constructor(private macrosConfig: MacrosConfig) {
super();
constructor(private macrosConfig: MacrosConfig, appRoot: string) {
super(appRoot);
}

moveAddons(appSrcDir: string, destDir: string): MovedPackageCache {
moveAddons(destDir: string): MovedPackageCache {
// start with the plain old app package
let origApp = this.getApp(appSrcDir);
let origApp = this.get(this.appRoot);

// discover the set of all packages that will need to be moved into the
// workspace
Expand All @@ -30,7 +30,6 @@ export class MovablePackageCache extends PackageCache {

export class MovedPackageCache extends PackageCache {
readonly app!: Package;
readonly appDestDir: string;
private commonSegmentCount: number;
readonly moved: Map<Package, Package> = new Map();
readonly unmovedAddons: Set<Package>;
Expand All @@ -43,14 +42,17 @@ export class MovedPackageCache extends PackageCache {
private origApp: Package,
private macrosConfig: MacrosConfig
) {
super();
// this is the initial appRoot, which we can't know until just below here
super('not-the-real-root');

// that gives us our common segment count, which enables localPath mapping
this.commonSegmentCount = movedSet.commonSegmentCount;

// so we can now determine where the app will go inside the workspace
this.appDestDir = this.localPath(origApp.root);
this.macrosConfig.packageMoved(origApp.root, this.appDestDir);
// so we can now determine where the app will go inside the workspace. THIS
// is where we fix 'not-the-real-root' from above.
this.appRoot = this.localPath(origApp.root);

this.macrosConfig.packageMoved(origApp.root, this.appRoot);

for (let originalPkg of movedSet.packages) {
// Update our rootCache so we don't need to rediscover moved packages
Expand Down
6 changes: 3 additions & 3 deletions packages/compat/src/prebuilt-addons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ export default class PrebuiltAddons implements Stage {
}

class RehomedPackageCache extends PackageCache {
constructor(private appSrcDir: string, private appDestDir: string) {
super();
constructor(private appSrcDir: string, appDestDir: string) {
super(appDestDir);
}
basedir(pkg: Package): string {
if (pkg.root === this.appSrcDir) {
return this.appDestDir;
return this.appRoot;
}
return super.basedir(pkg);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/compat/src/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ export default class CompatResolver implements Resolver {
}

absPathToRuntimePath(absPath: string, owningPackage?: { root: string; name: string }) {
let pkg = owningPackage || PackageCache.shared('embroider-stage3').ownerOfFile(absPath);
let pkg = owningPackage || PackageCache.shared('embroider-stage3', this.params.root).ownerOfFile(absPath);
if (pkg) {
let packageRuntimeName = pkg.name;
for (let [runtimeName, realName] of Object.entries(this.adjustImportsOptions.renamePackages)) {
Expand Down Expand Up @@ -454,7 +454,7 @@ export default class CompatResolver implements Resolver {
private tryHelper(path: string, from: string): Resolution | null {
let parts = path.split('@');
if (parts.length > 1 && parts[0].length > 0) {
let cache = PackageCache.shared('embroider-stage3');
let cache = PackageCache.shared('embroider-stage3', this.params.root);
let packageName = parts[0];
let renamed = this.adjustImportsOptions.renamePackages[packageName];
if (renamed) {
Expand Down Expand Up @@ -488,7 +488,7 @@ export default class CompatResolver implements Resolver {
private tryModifier(path: string, from: string): Resolution | null {
let parts = path.split('@');
if (parts.length > 1 && parts[0].length > 0) {
let cache = PackageCache.shared('embroider-stage3');
let cache = PackageCache.shared('embroider-stage3', this.params.root);
let packageName = parts[0];
let renamed = this.adjustImportsOptions.renamePackages[packageName];
if (renamed) {
Expand Down Expand Up @@ -527,7 +527,7 @@ export default class CompatResolver implements Resolver {
private tryComponent(path: string, from: string, withRuleLookup = true): Resolution | null {
let parts = path.split('@');
if (parts.length > 1 && parts[0].length > 0) {
let cache = PackageCache.shared('embroider-stage3');
let cache = PackageCache.shared('embroider-stage3', this.params.root);
let packageName = parts[0];
let renamed = this.adjustImportsOptions.renamePackages[packageName];
if (renamed) {
Expand Down
22 changes: 13 additions & 9 deletions packages/compat/src/v1-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { Node } from 'broccoli-node-api';
import { V1Config, WriteV1Config } from './v1-config';
import { WriteV1AppBoot, ReadV1AppBoot } from './v1-appboot';
import {
PackageCache,
TemplateCompiler,
TemplateCompilerPlugins,
AddonMeta,
Expand All @@ -23,7 +22,7 @@ import { writeJSONSync, ensureDirSync, copySync, readdirSync, pathExistsSync, ex
import AddToTree from './add-to-tree';
import DummyPackage, { OwningAddon } from './dummy-package';
import { TransformOptions } from '@babel/core';
import { isEmbroiderMacrosPlugin } from '@embroider/macros/src/node';
import { isEmbroiderMacrosPlugin, MacrosConfig } from '@embroider/macros/src/node';
import resolvePackagePath from 'resolve-package-path';
import Concat from 'broccoli-concat';
import mapKeys from 'lodash/mapKeys';
Expand All @@ -33,6 +32,7 @@ import prepHtmlbarsAstPluginsForUnwrap from './prepare-htmlbars-ast-plugins';
import { readFileSync } from 'fs';
import type { Options as HTMLBarsOptions } from 'ember-cli-htmlbars';
import semver from 'semver';
import { MovablePackageCache } from './moved-package-cache';

// This controls and types the interface between our new world and the classic
// v1 app instance.
Expand All @@ -51,20 +51,24 @@ export default class V1App {
// used to signal that this is a dummy app owned by a particular addon
owningAddon: Package | undefined;

static create(app: EmberAppInstance, packageCache: PackageCache): V1App {
static create(app: EmberAppInstance): V1App {
if (app.project.pkg.keywords?.includes('ember-addon')) {
// we are a dummy app, which is unfortunately weird and special
return new V1DummyApp(app, packageCache);
return new V1DummyApp(app);
} else {
return new V1App(app, packageCache);
return new V1App(app);
}
}

private _publicAssets: { [filePath: string]: string } = Object.create(null);
private _implicitScripts: string[] = [];
private _implicitStyles: string[] = [];

protected constructor(protected app: EmberAppInstance, protected packageCache: PackageCache) {}
packageCache: MovablePackageCache;

protected constructor(protected app: EmberAppInstance) {
this.packageCache = new MovablePackageCache(MacrosConfig.for(app, this.root), this.root);
}

// always the name from package.json. Not the one that apps may have weirdly
// customized.
Expand Down Expand Up @@ -742,9 +746,9 @@ function throwIfMissing<T>(
}

class V1DummyApp extends V1App {
constructor(app: EmberAppInstance, packageCache: PackageCache) {
super(app, packageCache);
this.owningAddon = new OwningAddon(this.app.project.root, packageCache);
constructor(app: EmberAppInstance) {
super(app);
this.owningAddon = new OwningAddon(this.app.project.root, this.packageCache);
this.packageCache.seed(this.owningAddon);
this.packageCache.seed(new DummyPackage(this.root, this.owningAddon, this.packageCache));
}
Expand Down
8 changes: 2 additions & 6 deletions packages/compat/src/v1-instance-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import V1App from './v1-app';
import V1Addon, { V1AddonConstructor } from './v1-addon';
import { pathExistsSync } from 'fs-extra';
import { AddonInstance, getOrCreate } from '@embroider/core';
import { MovablePackageCache } from './moved-package-cache';
import Options from './options';
import isEqual from 'lodash/isEqual';
import { MacrosConfig } from '@embroider/macros/src/node';

export default class V1InstanceCache {
static caches: WeakMap<object, V1InstanceCache> = new WeakMap();
Expand All @@ -28,12 +26,10 @@ export default class V1InstanceCache {
private addons: Map<string, V1Addon[]> = new Map();

app: V1App;
packageCache: MovablePackageCache;
orderIdx: number;

private constructor(oldApp: any, private options: Required<Options>) {
this.packageCache = new MovablePackageCache(MacrosConfig.for(oldApp));
this.app = V1App.create(oldApp, this.packageCache);
this.app = V1App.create(oldApp);
this.orderIdx = 0;

// no reason to do this on demand because oldApp already eagerly loaded
Expand Down Expand Up @@ -75,7 +71,7 @@ export default class V1InstanceCache {
private addAddon(addonInstance: AddonInstance) {
this.orderIdx += 1;
let Klass = this.adapterClass(addonInstance);
let v1Addon = new Klass(addonInstance, this.options, this.app, this.packageCache, this.orderIdx);
let v1Addon = new Klass(addonInstance, this.options, this.app, this.app.packageCache, this.orderIdx);
let pkgs = getOrCreate(this.addons, v1Addon.root, () => []);
pkgs.push(v1Addon);
addonInstance.addons.forEach(a => this.addAddon(a));
Expand Down
1 change: 1 addition & 0 deletions packages/compat/tests/audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe('audit', function () {
relocatedFiles: {},
resolvableExtensions,
emberNeedsModulesPolyfill: true,
appRoot: app.baseDir,
},
}),
},
Expand Down
1 change: 1 addition & 0 deletions packages/compat/tests/resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ describe('compat-resolver', function () {
relocatedFiles: {},
resolvableExtensions: ['.js', '.hbs'],
emberNeedsModulesPolyfill: false,
appRoot: appDir,
},
otherOptions.adjustImportsImports
),
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ export class AppBuilder<TreeNames> {

let colocationOptions: ColocationOptions = {
packageGuard: true,
appRoot: this.root,
};
babel.plugins.push([
require.resolve('@embroider/shared-internals/src/template-colocation-plugin'),
Expand Down Expand Up @@ -872,7 +873,7 @@ export class AppBuilder<TreeNames> {

async build(inputPaths: OutputPaths<TreeNames>) {
if (this.adapter.env !== 'production') {
this.macrosConfig.enableAppDevelopment(this.root);
this.macrosConfig.enablePackageDevelopment(this.root);
this.macrosConfig.enableRuntimeMode();
}
for (let pkgRoot of this.adapter.developingAddons()) {
Expand Down Expand Up @@ -1007,7 +1008,7 @@ export class AppBuilder<TreeNames> {
);
writeFileSync(
join(this.root, '_babel_filter_.js'),
babelFilterTemplate({ skipBabel: this.options.skipBabel }),
babelFilterTemplate({ skipBabel: this.options.skipBabel, appRoot: this.root }),
'utf8'
);
}
Expand Down Expand Up @@ -1468,8 +1469,8 @@ function stringOrBufferEqual(a: string | Buffer, b: string | Buffer): boolean {

const babelFilterTemplate = compile(`
const { babelFilter } = require('@embroider/core');
module.exports = babelFilter({{{json-stringify skipBabel}}});
`) as (params: { skipBabel: Options['skipBabel'] }) => string;
module.exports = babelFilter({{{json-stringify skipBabel}}}, "{{{js-string-escape appRoot}}}");
`) as (params: { skipBabel: Options['skipBabel']; appRoot: string }) => string;

// meta['renamed-modules'] has mapping from classic filename to real filename.
// This takes that and converts it to the inverst mapping from real import path
Expand Down
Loading

0 comments on commit f39b6f0

Please sign in to comment.