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 importing of template-only components in V2 addons #1126

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/addon-dev/package.json
Expand Up @@ -25,6 +25,7 @@
"dependencies": {
"@embroider/shared-internals": "^1.6.0",
"@rollup/pluginutils": "^4.1.1",
"assert-never": "^1.2.1",
"fs-extra": "^10.0.0",
"minimatch": "^3.0.4",
"rollup-plugin-copy-assets": "^2.0.3",
Expand Down
134 changes: 107 additions & 27 deletions packages/addon-dev/src/rollup-hbs-plugin.ts
@@ -1,47 +1,127 @@
import { createFilter } from '@rollup/pluginutils';
import type { Plugin } from 'rollup';
import type {
Plugin,
PluginContext,
CustomPluginOptions,
ResolvedId,
} from 'rollup';
import { readFileSync } from 'fs';
import { hbsToJS } from '@embroider/shared-internals';
import assertNever from 'assert-never';
import { parse as pathParse } from 'path';

export default function rollupHbsPlugin(): Plugin {
const filter = createFilter('**/*.hbs');

return {
name: 'rollup-hbs-plugin',
async resolveId(source: string, importer: string | undefined, options) {
const resolution = await this.resolve(source, importer, {
let resolution = await this.resolve(source, importer, {
skipSelf: true,
...options,
});

const id = resolution?.id;

if (!filter(id)) return null;

// This creates an `*.hbs.js` that we will populate in `load()` hook.
return {
...resolution,
id: id + '.js',
meta: {
'rollup-hbs-plugin': {
originalId: id,
},
},
};
if (!resolution) {
return maybeSynthesizeComponentJS(this, source, importer, options);
} else {
return maybeRewriteHBS(resolution);
}
},
load(id: string) {
const meta = this.getModuleInfo(id)?.meta;
const originalId = meta?.['rollup-hbs-plugin']?.originalId;

if (!originalId) {
load(id: string) {
const meta = getMeta(this, id);
if (!meta) {
return;
}

let input = readFileSync(originalId, 'utf8');
let code = hbsToJS(input);
return {
code,
};
switch (meta.type) {
case 'template':
let input = readFileSync(meta.originalId, 'utf8');
let code = hbsToJS(input);
return {
code,
};
case 'template-only-component-js':
return {
code: templateOnlyComponent,
};
default:
assertNever(meta);
}
},
};
}

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

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

function getMeta(context: PluginContext, id: string): Meta | null {
const meta = context.getModuleInfo(id)?.meta?.['rollup-hbs-plugin'];
if (meta) {
return meta as Meta;
} else {
return null;
}
}

function correspondingTemplate(filename: string): string {
let { ext } = pathParse(filename);
return filename.slice(0, filename.length - ext.length) + '.hbs';
}

async function maybeSynthesizeComponentJS(
context: PluginContext,
source: string,
importer: string | undefined,
options: { custom?: CustomPluginOptions; isEntry: boolean }
) {
let templateResolution = await context.resolve(
correspondingTemplate(source),
importer,
{
skipSelf: true,
...options,
}
);
if (!templateResolution) {
return null;
}
// we're trying to resolve a JS module but only the corresponding HBS
// file exists. Synthesize the template-only component JS.
return {
id: templateResolution.id.replace(/\.hbs$/, '.js'),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is way simpler than what i was doing 🤔

meta: {
'rollup-hbs-plugin': {
type: 'template-only-component-js',
},
},
};
}

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

function maybeRewriteHBS(resolution: ResolvedId) {
if (!hbsFilter(resolution.id)) {
return null;
}

// This creates an `*.hbs.js` that we will populate in `load()` hook.
return {
...resolution,
id: resolution.id + '.js',
meta: {
'rollup-hbs-plugin': {
type: 'template',
originalId: resolution.id,
},
},
};
}
51 changes: 40 additions & 11 deletions packages/addon-dev/src/rollup-public-entrypoints.ts
@@ -1,26 +1,55 @@
import walkSync from 'walk-sync';
import type { Plugin } from 'rollup';
import { join } from 'path';
import minimatch from 'minimatch';

import type { Plugin } from 'rollup';
import { pathExistsSync } from 'fs-extra';

function normalizeFileExt(fileName: string) {
return fileName.replace(/\.ts|\.gts|\.gjs$/, '.js');
return fileName.replace(/\.ts|\.hbs|\.gts|\.gjs$/, '.js');
}

const hbsPattern = '**/*.hbs';

export default function publicEntrypoints(args: {
srcDir: string;
include: string[];
}): Plugin {
return {
name: 'addon-modules',
buildStart() {
for (let name of walkSync(args.srcDir, {
globs: args.include,
})) {
this.emitFile({
type: 'chunk',
id: join(args.srcDir, name),
fileName: normalizeFileExt(name),
});
async buildStart() {
let matches = walkSync(args.srcDir, {
globs: [...args.include, hbsPattern],
});

for (let name of matches) {
if (args.include.some((pattern) => minimatch(name, pattern))) {
// anything that matches one of the user's patterns is definitely emitted
this.emitFile({
type: 'chunk',
id: join(args.srcDir, name),
fileName: normalizeFileExt(name),
});
} else {
// this file didn't match one of the user's patterns, so it must match
// our hbsPattern. Infer the possible existence of a synthesized
// template-only component JS file and test whether that file would
// match the user's patterns.
let normalizedName = normalizeFileExt(name);
let id = join(args.srcDir, normalizedName);
if (
args.include.some((pattern) =>
minimatch(normalizedName, pattern)
) &&
!pathExistsSync(id)
) {
this.emitFile({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah,. This was probably what i was missing (and maybe all the changes in this file).

Because i was so focused on the other plugin, i didn't remember how in interacts with this plugin.

type: 'chunk',
id,
fileName: normalizedName,
});
}
}
}
},
};
Expand Down
22 changes: 14 additions & 8 deletions tests/scenarios/package.json
Expand Up @@ -21,37 +21,43 @@
},
"license": "MIT",
"devDependencies": {
"@babel/core": "^7.17.5",
"@babel/plugin-proposal-class-properties": "^7.16.7",
"@babel/plugin-proposal-decorators": "^7.17.2",
"@babel/preset-env": "^7.16.11",
"@ember/string": "^1.0.0",
"@embroider/macros": "1.6.0",
"@embroider/addon-shim": "1.6.0",
"@embroider/router": "1.6.0",
"@embroider/util": "1.6.0",
"@rollup/plugin-babel": "^5.3.1",
"bootstrap": "^4.3.1",
"broccoli-funnel": "^3.0.5",
"broccoli-merge-trees": "^3.0.2",
"broccoli-persistent-filter": "^3.1.2",
"ember-bootstrap": "^5.0.0",
"ember-cli": "~3.28.0",
"ember-cli-3.16": "npm:ember-cli@~3.16.0",
"ember-cli-3.24": "npm:ember-cli@~3.24.0",
"ember-cli": "~3.28.0",
"ember-cli-beta": "npm:ember-cli@beta",
"ember-cli-htmlbars-inline-precompile": "^2.1.0",
"ember-cli-fastboot": "^3.2.0",
"ember-cli-htmlbars-3": "npm:ember-cli-htmlbars@3",
"ember-cli-htmlbars-inline-precompile": "^2.1.0",
"ember-cli-latest": "npm:ember-cli@latest",
"ember-cli-fastboot": "^3.2.0",
"ember-composable-helpers": "^4.4.1",
"ember-data": "~3.28.0",
"ember-data-3.16": "npm:ember-data@~3.16.0",
"ember-data-3.24": "npm:ember-data@~3.24.0",
"ember-data": "~3.28.0",
"ember-data-latest": "npm:ember-data@latest",
"ember-engines": "^0.8.17",
"ember-inline-svg": "^0.2.1",
"ember-source-latest": "npm:ember-source@latest",
"ember-source-beta": "npm:ember-source@beta",
"ember-source": "~3.28.0",
"ember-source-3.16": "npm:ember-source@~3.16.0",
"ember-source-3.24": "npm:ember-source@~3.24.0",
"ember-source": "~3.28.0",
"ember-truth-helpers": "^3.0.0"
"ember-source-beta": "npm:ember-source@beta",
"ember-source-latest": "npm:ember-source@latest",
"ember-truth-helpers": "^3.0.0",
"rollup": "^2.69.1"
},
"volta": {
"node": "14.16.1",
Expand Down