Skip to content

Commit

Permalink
Merge pull request #2256 from embroider-build/staticAddonTrees-error
Browse files Browse the repository at this point in the history
remove staticAddonTrees and staticAddonTestSupportTrees
  • Loading branch information
ef4 authored Feb 11, 2025
2 parents 89cc372 + 343fb36 commit 5de6e89
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 124 deletions.
80 changes: 32 additions & 48 deletions packages/compat/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,6 @@ import type { PackageRules } from './dependency-rules';
// the cost of slower or bigger builds. As you eliminate sources of legacy
// behavior you can benefit from the more aggressive modes.
export default interface Options extends CoreOptions {
/**
* Controls whether your addon's "addon" trees should be resolved statically
* at build time.
*
* Note: This setting will be removed in the next version of Embroider and
* will effectively default to true
*
* false (the current default): implies maximum backward compatibility at
* the cost of bigger builds. In this mode, we force every file into the
* Ember app, which is the legacy behavior.
*
* true: produces smaller builds. The addon files must be imported from
* somewhere we can statically see during the build. In this mode, your app
* will only include files that are actually imported from somewhere.
*
* Commentary: most v1 addons already work well with this set to true, because
* they tend to either offer Javascript that users are supposed to directly
* `import` or components / helpers / services that get directly imported and
* re-exported by code in App Javascript. The exceptions are addons that do
* runtime shenanigans with `require` or scoped runtime resolutions.
*
* To workaround an addon that is preventing you from enabling this flag, you
* can use addonDependencyRules.
*/
staticAddonTrees?: boolean;

// Controls whether your addon's "addonTestSupport" trees should be resolved
// statically at build time.
//
// false (the default): implies maximum backward compatibility at the cost
// of bigger builds. All test support files will be forced into your Ember
// app, which is the legacy behavior.
//
// true: produces smaller builds. Only files that are explicitly imported
// will end up in your app.
//
// Commentary: this is analogous to staticAddonTrees and the same guidelines
// apply.
staticAddonTestSupportTrees?: boolean;

// Allows you to override how specific addons will build. Like:
//
// import V1Addon from '@embroider/compat'; let compatAdapters = new Map();
Expand Down Expand Up @@ -126,16 +86,40 @@ export function optionsWithDefaults(options?: Options): CompatOptionsType {
);
}

if (!options?.staticAddonTrees) {
console.log(
`The setting 'staticAddonTrees' will default to true in the next version of Embroider and can't be turned off. To prepare for this you should set 'staticAddonTrees: true' in your Embroider config.`
);
if ((options as any)?.staticEmberSource !== undefined) {
if ((options as any).staticEmberSource === false) {
throw new Error(
`You have set 'staticEmberSource' to 'false' in your Embroider options. This option has been removed is always considered to have the value 'true'. Please remove this setting to continue.`
);
} else {
console.log(
`You have set 'staticEmberSource' in your Embroider options. This can safely be removed now and it defaults to true.`
);
}
}

if (!options?.staticAddonTestSupportTrees) {
console.log(
`The setting 'staticAddonTestSupportTrees' will default to true in the next version of Embroider and can't be turned off. To prepare for this you should set 'staticAddonTestSupportTrees: true' in your Embroider config.`
);
if ((options as any)?.staticAddonTrees !== undefined) {
if ((options as any).staticAddonTrees === false) {
throw new Error(
`You have set 'staticAddonTrees' to 'false' in your Embroider options. This option has been removed is always considered to have the value 'true'. Please remove this setting to continue.`
);
} else {
console.log(
`You have set 'staticAddonTrees' in your Embroider options. This can safely be removed now and it defaults to true.`
);
}
}

if ((options as any)?.staticAddonTestSupportTrees !== undefined) {
if ((options as any).staticAddonTestSupportTrees === false) {
throw new Error(
`You have set 'staticAddonTestSupportTrees' to 'false' in your Embroider options. This option has been removed is always considered to have the value 'true'. Please remove this setting to continue.`
);
} else {
console.log(
`You have set 'staticAddonTestSupportTrees' in your Embroider options. This can safely be removed now and it defaults to true.`
);
}
}

if ((options as any)?.skipBabel !== undefined) {
Expand Down
48 changes: 8 additions & 40 deletions packages/compat/src/v1-addon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,39 +759,20 @@ export default class V1Addon {
// .hbs.js
templateExtensions: ['.hbs', '.hbs.js'],
});
if (this.addonOptions.staticAddonTrees) {
if (this.isEngine()) {
// even when staticAddonTrees is enabled, engines may have a router map
// that needs to be dynamically resolved.
let hasRoutesModule = false;

tree = new ObserveTree(tree, outputDir => {
hasRoutesModule = existsSync(resolve(outputDir, 'routes.js'));
});
built.dynamicMeta.push(() => ({
'implicit-modules': hasRoutesModule ? ['./routes.js'] : [],
}));
}
} else {
let filenames: string[] = [];
let templateOnlyComponentNames: string[] = [];

tree = new ObserveTree(tree, outputDir => {
filenames = walkSync(outputDir, { globs: ['**/*.js', '**/*.hbs'] })
.map(f => `./${f.replace(/\.js$/i, '')}`)
.filter(notColocatedTemplate);
});
if (this.isEngine()) {
// even when staticAddonTrees is enabled, engines may have a router map
// that needs to be dynamically resolved.
let hasRoutesModule = false;

templateOnlyComponents = new ObserveTree(templateOnlyComponents, outputDir => {
templateOnlyComponentNames = walkSync(outputDir, { globs: ['**/*.js'] }).map(
f => `./${f.replace(/\.js$/i, '')}`
);
tree = new ObserveTree(tree, outputDir => {
hasRoutesModule = existsSync(resolve(outputDir, 'routes.js'));
});

built.dynamicMeta.push(() => ({
'implicit-modules': filenames.concat(templateOnlyComponentNames),
'implicit-modules': hasRoutesModule ? ['./routes.js'] : [],
}));
}

built.trees.push(tree);
built.trees.push(templateOnlyComponents);
}
Expand Down Expand Up @@ -863,15 +844,6 @@ export default class V1Addon {
addonTestSupportTree = this.transpile(this.stockTree('addon-test-support'));
}
if (addonTestSupportTree) {
if (!this.addonOptions.staticAddonTestSupportTrees) {
let filenames: string[] = [];
addonTestSupportTree = new ObserveTree(addonTestSupportTree, outputPath => {
filenames = walkSync(outputPath, { globs: ['**/*.js', '**/*.hbs'] }).map(f => `./${f.replace(/.js$/i, '')}`);
});
built.dynamicMeta.push(() => ({
'implicit-test-modules': filenames,
}));
}
built.trees.push(addonTestSupportTree);
}
}
Expand Down Expand Up @@ -1147,10 +1119,6 @@ function babelPluginAllowedInStage1(plugin: PluginItem) {
return true;
}

function notColocatedTemplate(path: string) {
return !/^\.\/components\/.*\.hbs$/.test(path);
}

const markedEmptyTree = new UnwatchedDir(process.cwd());

const stubbedSuper = () => {
Expand Down
15 changes: 1 addition & 14 deletions tests/scenarios/compat-exclude-dot-files-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ appScenarios
module.exports = function (defaults) {
let app = new EmberApp(defaults, {});
return maybeEmbroider(app, {
staticAddonTrees: false,
});
return maybeEmbroider(app);
};
`,
app: {
Expand Down Expand Up @@ -96,16 +94,5 @@ appScenarios
return true;
});
});

test('dot files are not included as addon implicit-modules', function () {
// Dot files should exist on disk
expectFile('./node_modules/my-addon/.fooaddon.js').exists();
expectFile('./node_modules/my-addon/baraddon.js').exists();

let myAddonPackage = expectFile('./node_modules/my-addon/package.json').json();

// dot files are not included as implicit-modules
myAddonPackage.get(['ember-addon', 'implicit-modules']).deepEquals(['./baraddon']);
});
});
});
8 changes: 2 additions & 6 deletions tests/scenarios/compat-template-colocation-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ scenarios
let app = new EmberApp(defaults, {});
return prebuild(app, {
staticInvokables: false,
staticAddonTrees: false,
});
};
`,
Expand Down Expand Up @@ -229,12 +228,9 @@ module('Integration | Component | addon-component-one', function (hooks) {
);
});

test(`addon's colocated components are correct in implicit-modules`, function () {
test(`addon's colocated components are not in implicit-modules`, function () {
let assertFile = expectFile('./node_modules/my-addon/package.json').json();
assertFile.get(['ember-addon', 'implicit-modules']).includes('./components/component-one');
assertFile.get(['ember-addon', 'implicit-modules']).includes('./components/component-two');
assertFile.get(['ember-addon', 'implicit-modules']).doesNotInclude('./components/component-one.hbs');
assertFile.get(['ember-addon', 'implicit-modules']).doesNotInclude('./components/component-two.hbs');
assertFile.get(['ember-addon', 'implicit-modules']).equals(undefined);
});

// TODO running pnpm test in this scenario is causing rollup to build things in a strange order
Expand Down
16 changes: 0 additions & 16 deletions tests/scenarios/stage1-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ appScenarios
});
return maybeEmbroider(app, {
staticAddonTestSupportTrees: false,
staticAddonTrees: false,
staticComponents: false,
staticHelpers: false,
staticModifiers: false,
Expand Down Expand Up @@ -130,20 +128,6 @@ appScenarios
.get('ember-addon.app-js')
.deepEquals({ './components/hello-world.js': './_app_/components/hello-world.js' });

myAddonPkg
.get('ember-addon.implicit-modules')
.includes(
'./components/hello-world',
'staticAddonTrees is off so we should include the component implicitly'
);

myAddonPkg
.get('ember-addon.implicit-modules')
.includes(
'./templates/components/hello-world.hbs',
'staticAddonTrees is off so we should include the template implicitly'
);

myAddonPkg.get('ember-addon.version').deepEquals(2);
});

Expand Down

0 comments on commit 5de6e89

Please sign in to comment.