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

addon-dev: incremental updates to output #1855

Merged
merged 1 commit into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/addon-dev/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"fs-extra": "^10.0.0",
"minimatch": "^3.0.4",
"rollup-plugin-copy-assets": "^2.0.3",
"rollup-plugin-delete": "^2.0.0",
"walk-sync": "^3.0.0",
"yargs": "^17.0.1"
},
Expand Down
4 changes: 1 addition & 3 deletions packages/addon-dev/src/rollup-gjs-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { createFilter } from '@rollup/pluginutils';
import type { Plugin } from 'rollup';
import { readFileSync } from 'fs';
import { Preprocessor } from 'content-tag';

const PLUGIN_NAME = 'rollup-gjs-plugin';
Expand All @@ -14,11 +13,10 @@ export default function rollupGjsPlugin(
return {
name: PLUGIN_NAME,

load(id: string) {
transform(input: string, id: string) {
simonihmig marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ef4 I noticed that now the gjs plugin must come before the babel plugin in the rollup config. That would make this a breaking change

if (!gjsFilter(id)) {
return null;
}
let input = readFileSync(id, 'utf8');
let code = processor.process(input, {
filename: id,
inline_source_map,
Expand Down
144 changes: 75 additions & 69 deletions packages/addon-dev/src/rollup-hbs-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import type { Plugin, PluginContext } from 'rollup';
import { createFilter } from '@rollup/pluginutils';
import type { Plugin, PluginContext, CustomPluginOptions } from 'rollup';
import { readFileSync } from 'fs';
import { correspondingTemplate, hbsToJS } from '@embroider/core';
import minimatch from 'minimatch';
import {
hbsToJS,
templateOnlyComponentSource,
needsSyntheticComponentJS,
syntheticJStoHBS,
} from '@embroider/core';
import { extname } from 'path';

const hbsFilter = createFilter('**/*.hbs?([?]*)');

export default function rollupHbsPlugin({
excludeColocation,
Expand All @@ -12,48 +19,88 @@ export default function rollupHbsPlugin({
return {
name: 'rollup-hbs-plugin',
async resolveId(source: string, importer: string | undefined, options) {
if (options.custom?.embroider?.isExtensionSearch) {
return null;
}

let resolution = await this.resolve(source, importer, {
skipSelf: true,
...options,
});

if (resolution) {
return resolution;
} else {
return maybeSynthesizeComponentJS(
this,
source,
importer,
options,
excludeColocation
if (!resolution && extname(source) === '') {
resolution = await this.resolve(source + '.hbs', importer, {
skipSelf: true,
});
}

if (!resolution) {
let hbsSource = syntheticJStoHBS(source);
if (hbsSource) {
resolution = await this.resolve(hbsSource, importer, {
skipSelf: true,
custom: {
embroider: {
isExtensionSearch: true,
},
},
});
}

if (!resolution) {
return null;
}
}

if (resolution && resolution.id.endsWith('.hbs')) {
let isExcluded = excludeColocation?.some((glob) =>
minimatch(resolution!.id, glob)
);
if (isExcluded) {
return resolution;
}
}

let syntheticId = needsSyntheticComponentJS(source, resolution.id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we do not have a packageCache here

if (syntheticId) {
this.addWatchFile(source);
return {
id: syntheticId,
meta: {
'rollup-hbs-plugin': {
type: 'template-only-component-js',
},
},
};
}
},

load(id: string) {
if (hbsFilter(id)) {
return getHbsToJSCode(id);
}
let meta = getMeta(this, id);
if (meta) {
if (meta?.type === 'template-js') {
const hbsFile = id.replace(/\.js$/, '.hbs');
return getHbsToJSCode(hbsFile);
}
if (getMeta(this, id)?.type === 'template-only-component-js') {
this.addWatchFile(id);
return {
code: templateOnlyComponent,
code: templateOnlyComponentSource(),
};
}
},

transform(code: string, id: string) {
let hbsFilename = id.replace(/\.\w{1,3}$/, '') + '.hbs';
if (hbsFilename !== id) {
this.addWatchFile(hbsFilename);
if (getMeta(this, id)?.type === 'template-only-component-js') {
this.addWatchFile(id);
}
}
if (!hbsFilter(id)) {
return null;
}
return hbsToJS(code);
},
};
}
patricklx marked this conversation as resolved.
Show resolved Hide resolved

const templateOnlyComponent =
`import templateOnly from '@ember/component/template-only';\n` +
`export default templateOnly();\n`;

type Meta = {
type: 'template-only-component-js' | 'template-js';
type: 'template-only-component-js';
};

function getMeta(context: PluginContext, id: string): Meta | null {
Expand All @@ -64,44 +111,3 @@ function getMeta(context: PluginContext, id: string): Meta | null {
return null;
}
}

function getHbsToJSCode(file: string): { code: string } {
let input = readFileSync(file, 'utf8');
let code = hbsToJS(input);
return {
code,
};
}

async function maybeSynthesizeComponentJS(
context: PluginContext,
source: string,
importer: string | undefined,
options: { custom?: CustomPluginOptions; isEntry: boolean },
excludeColocation: string[] | undefined
) {
let hbsFilename = correspondingTemplate(source);
let templateResolution = await context.resolve(hbsFilename, importer, {
skipSelf: true,
...options,
});
if (!templateResolution) {
return null;
}
let type = excludeColocation?.some((glob) => minimatch(hbsFilename, glob))
? 'template-js'
: 'template-only-component-js';
// we're trying to resolve a JS module but only the corresponding HBS
// file exists. Synthesize the JS. The meta states if the hbs corresponds
// to a template-only component or a simple template like a route template.
return {
id: templateResolution.id.replace(/\.hbs$/, '.js'),
meta: {
'rollup-hbs-plugin': {
type,
},
},
};
}

const hbsFilter = createFilter('**/*.hbs');
79 changes: 79 additions & 0 deletions packages/addon-dev/src/rollup-incremental-plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import walkSync from 'walk-sync';
import { rmSync } from 'fs';
import { join } from 'path';
import type { Plugin } from 'rollup';
import { existsSync } from 'fs-extra';

export default function incremental(): Plugin {
const generatedAssets = new Map();
const generatedFiles = new Set<string>();

function isEqual(v1: string | Uint8Array, v2: string | Uint8Array): boolean {
if (typeof v1 === 'string' && typeof v2 === 'string') {
return v1 === v2;
}
if (Buffer.isBuffer(v1) && Buffer.isBuffer(v2)) {
return v1.equals(v2);
}
return false;
}

let firstTime = true;

function initGeneratedFiles(outDir: string) {
if (existsSync(outDir)) {
const files = walkSync(outDir, {
globs: ['*/**'],
directories: false,
});
for (const file of files) {
generatedFiles.add(file);
}
}
}

function deleteRemovedFiles(bundle: Record<string, any>, outDir: string) {
for (const file of generatedFiles) {
if (!bundle[file]) {
generatedAssets.delete(file);
rmSync(join(outDir, file));
}
}
generatedFiles.clear();
for (const file of Object.keys(bundle)) {
generatedFiles.add(file);
}
}

function syncFiles(bundle: Record<string, any>) {
for (const checkKey of Object.keys(bundle)) {
if (bundle[checkKey]) {
let module = bundle[checkKey] as any;
let code = module.source || module.code;
if (
generatedAssets.has(checkKey) &&
isEqual(code, generatedAssets.get(checkKey))
) {
delete bundle[checkKey];
} else {
generatedAssets.set(checkKey, code);
}
}
}
}

return {
name: 'incremental',
generateBundle(options, bundle) {
if (firstTime) {
firstTime = false;
initGeneratedFiles(options.dir!);
}
if (existsSync(options.dir!)) {
deleteRemovedFiles(bundle, options.dir!);
}

syncFiles(bundle);
},
};
}
2 changes: 1 addition & 1 deletion packages/addon-dev/src/rollup-public-entrypoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default function publicEntrypoints(args: {
}): Plugin {
return {
name: 'addon-modules',

async buildStart() {
let matches = walkSync(args.srcDir, {
globs: [...args.include, '**/*.hbs', '**/*.ts', '**/*.gts', '**/*.gjs'],
Expand All @@ -23,7 +24,6 @@ export default function publicEntrypoints(args: {

for (let name of matches) {
this.addWatchFile(path.join(args.srcDir, name));

// the matched file, but with the extension swapped with .js
let normalizedName = normalizeFileExt(name);

Expand Down
12 changes: 6 additions & 6 deletions packages/addon-dev/src/rollup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ import { default as hbs } from './rollup-hbs-plugin';
import { default as gjs } from './rollup-gjs-plugin';
import { default as publicEntrypoints } from './rollup-public-entrypoints';
import { default as appReexports } from './rollup-app-reexports';
import type { Options as DelOptions } from 'rollup-plugin-delete';
import { default as clean } from 'rollup-plugin-delete';
patricklx marked this conversation as resolved.
Show resolved Hide resolved
import { default as keepAssets } from './rollup-keep-assets';
import { default as dependencies } from './rollup-addon-dependencies';
import {
default as publicAssets,
type PublicAssetsOptions,
} from './rollup-public-assets';
import { default as clean } from './rollup-incremental-plugin';
import type { Plugin } from 'rollup';

export class Addon {
Expand Down Expand Up @@ -64,10 +63,11 @@ export class Addon {
return gjs(options);
}

// By default rollup does not clear the output directory between builds. This
// does that.
clean(options: DelOptions) {
return clean({ targets: `${this.#destDir}/*`, ...options });
// this does incremental updates to the dist files and also deletes files that are not part of the generated bundle
// rollup already supports incremental transforms of files,
// this extends it to the dist files
clean() {
return clean();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we consider this a breaking change? If not, this PR could target the stable branch, not?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we consider this breaking, we could still release this on stable as a minor version, by exposing this as a separate plugin, and updating the blueprint to use the new one instead of addon.clean()! 🤔

}

// V2 Addons are allowed to contain imports of .css files. This tells rollup
Expand Down
13 changes: 5 additions & 8 deletions packages/shared-internals/src/colocation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,13 @@ export function syntheticJStoHBS(source: string): string | null {
return null;
}

export function needsSyntheticComponentJS(
requestedSpecifier: string,
foundFile: string,
packageCache: Pick<PackageCache, 'ownerOfFile'>
): string | null {
export function needsSyntheticComponentJS(requestedSpecifier: string, foundFile: string): string | null {
requestedSpecifier = cleanUrl(requestedSpecifier);
foundFile = cleanUrl(foundFile);
if (
discoveredImplicitHBS(requestedSpecifier, foundFile) &&
!foundFile.split(sep).join('/').endsWith('/template.hbs') &&
!correspondingJSExists(foundFile) &&
isInComponents(foundFile, packageCache)
!correspondingJSExists(foundFile)
) {
return foundFile.slice(0, -3) + 'js';
}
Expand All @@ -41,7 +36,9 @@ function correspondingJSExists(id: string): boolean {
return ['js', 'ts'].some(ext => existsSync(id.slice(0, -3) + ext));
}

export function isInComponents(id: string, packageCache: Pick<PackageCache, 'ownerOfFile'>) {
export function isInComponents(url: string, packageCache: Pick<PackageCache, 'ownerOfFile'>) {
const id = cleanUrl(url);

const pkg = packageCache.ownerOfFile(id);
if (!pkg?.isV2App()) {
return false;
Expand Down
6 changes: 3 additions & 3 deletions packages/vite/src/esbuild-resolver.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Plugin as EsBuildPlugin, OnLoadResult, PluginBuild, ResolveResult } from 'esbuild';
import { transform } from '@babel/core';
import { ResolverLoader, virtualContent, needsSyntheticComponentJS } from '@embroider/core';
import { ResolverLoader, virtualContent, needsSyntheticComponentJS, isInComponents } from '@embroider/core';
import { readFileSync } from 'fs-extra';
import { EsBuildModuleRequest } from './esbuild-request';
import assertNever from 'assert-never';
Expand Down Expand Up @@ -91,8 +91,8 @@ export function esBuildResolver(): EsBuildPlugin {
});

if (result.errors.length === 0 && !result.external) {
let syntheticPath = needsSyntheticComponentJS(path, result.path, resolverLoader.resolver.packageCache);
if (syntheticPath) {
let syntheticPath = needsSyntheticComponentJS(path, result.path);
if (syntheticPath && isInComponents(result.path, resolverLoader.resolver.packageCache)) {
return { path: syntheticPath, namespace: 'embroider-template-only-component' };
}
}
Expand Down
Loading
Loading