Skip to content

Commit

Permalink
It works
Browse files Browse the repository at this point in the history
format

Parse errors from content-tag and display them to the user as blocking errors before they can get more diagnostics

transform-manager.ts, remove unneeded change

rewrite-module: cleanup

formatting

Upgrade content-tag to 1.2.2

Use type provided by content-tag

Acknowledge that content-tag works in bytes, not characters

prettier
  • Loading branch information
NullVoxPopuli committed Mar 8, 2024
1 parent 5b5a294 commit 71b849c
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 34 deletions.
8 changes: 8 additions & 0 deletions packages/core/src/common/transform-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ export default class TransformManager {
);
}

// When we have syntax errors we get _too many errors_
// if we have an issue with <template> tranformation, we should
// make the user fix their syntax before revealing all the other errors.
let glint = allDiagnostics.filter((diagnostic) => 'isGlintTransformDiagnostic' in diagnostic);
if (glint.length) {
return this.ts.sortAndDeduplicateDiagnostics(glint);
}

return this.ts.sortAndDeduplicateDiagnostics(allDiagnostics);
}

Expand Down
113 changes: 109 additions & 4 deletions packages/core/src/transform/template/rewrite-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,38 @@ function calculateCorrelatedSpans(
let errors: Array<TransformError> = [];
let partialSpans: Array<PartialCorrelatedSpan> = [];

let { ast, emitMetadata } = parseScript(ts, script, environment);
let { ast, emitMetadata, error } = parseScript(ts, script, environment);

if (error) {
if (typeof error === 'string') {
errors.push({
message: error,
location: { start: 0, end: script.contents.length - 1 },
source: script,
});
} else if ('raw' in error) {
// these lines exclude the line with the error, because
// adding the column offset will get us on to the line with the error
let lines = script.contents.split('\n').slice(0, error.line);
let start = lines.reduce((sum, line) => sum + line.length, 0) + error.column - 1;
let end = start + 1;

errors.push({
// we have to show the "help" because content-tag has different line numbers
// than we are able to discern ourselves.
message: error.message + '\n\n' + error.help,
source: script,
location: {
start,
end,
},
});
}

// We've hit a parsing error, so we need to immediately return as the parsed
// document must be correct before we can continue.
return { errors, directives, partialSpans };
}

ts.transform(ast, [
(context) =>
Expand Down Expand Up @@ -103,9 +134,21 @@ function calculateCorrelatedSpans(
return { errors, directives, partialSpans };
}

type ParseError =
| string
| {
message: string;
line: number;
column: number;
file: string;
help: string;
raw: string;
};

type ParseResult = {
ast: ts.SourceFile;
emitMetadata: WeakMap<ts.Node, GlintEmitMetadata>;
error?: ParseError;
};

function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironment): ParseResult {
Expand All @@ -116,15 +159,77 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
void emitMetadata.set(node, Object.assign(emitMetadata.get(node) ?? {}, data));

let { preprocess, transform } = environment.getConfigForExtension(extension) ?? {};
let preprocessed = preprocess?.(contents, filename) ?? { contents };

let original: {
contents: string;
data?: {
// SAFETY: type exists elsewhere (the environments)
templateLocations: any[];
};
} = { contents, data: { templateLocations: [] } };

let preprocessed = original;
let error: ParseError | undefined;

try {
preprocessed = preprocess?.(contents, filename) ?? original;
} catch (e) {
if (typeof e === 'object' && e !== null) {
// Parse Errors from the rust parser
if ('source_code' in e) {
// We remove the blank links in the error because swc
// pads errors with a leading and trailing blank line.
// the error is typically meant for the terminal, so making it
// stand out a bit more is a good, but that's more a presentation
// concern than just pure error information (which is what we need).
// @ts-expect-error object / property narrowing isn't available until TS 5.1
let lines = e.source_code.split('\n').filter(Boolean);
// Example:
// ' × Unexpected eof'
// ' ╭─[/home/nullvoxpopuli/Development/OpenSource/glint/test-packages/ts-template-imports-app/src/index.gts:6:1]'
// ' 6 │ '
// ' 7 │ export const X = <tem'
// ' ╰────'
let raw = lines.join('\n');
let message = lines[0].replace('×', '').trim();
let info = lines[1];
// a filename may have numbers in it, so we want to remove the filename
// before regex searching for numbers at the end of this line
let strippedInfo = info.replace(filename, '');
let matches = [...strippedInfo.matchAll(/\d+/g)];
let line = parseInt(matches[0][0], 10);
let column = parseInt(matches[1][0], 10);
let help = lines.slice(1).join('\n');

error = {
raw,
message,
line,
column,
file: filename,
help,
};
} else {
error = `${e}`;
}
} else {
error = `${e}`;
}
}

let ast = ts.createSourceFile(
filename,
// contents will be transformed and placeholder'd content
// or, in the event of a parse error, the original content
// in which case, TS will report a ton of errors about some goofy syntax.
// We'll want to ignore all of that and only display our parsing error from content-tag.
preprocessed.contents,
ts.ScriptTarget.Latest,
true // setParentNodes
);

if (transform) {
// Only transform if we don't have a parse error
if (!error && transform) {
let { transformed } = ts.transform(ast, [
(context) => transform!(preprocessed.data, { ts, context, setEmitMetadata }),
]);
Expand All @@ -133,7 +238,7 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
ast = transformed[0];
}

return { ast, emitMetadata };
return { ast, emitMetadata, error };
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,54 +1,91 @@
import { GlintExtensionPreprocess } from '@glint/core/config-types';
import { parseTemplates } from 'ember-template-imports';
import { GLOBAL_TAG, PreprocessData, TemplateLocation } from './common';

const TEMPLATE_START = `[${GLOBAL_TAG}\``;
const TEMPLATE_END = '`]';

// content-tag 1.2.2:
// The current file is a CommonJS module whose imports will produce 'require' calls;
// however, the referenced file is an ECMAScript module and cannot be imported with 'require'.
// Consider writing a dynamic 'import("content-tag")' call instead.
// To convert this file to an ECMAScript module, change its file extension to '.mts',
// or add the field `"type": "module"` to 'glint/packages/environment-ember-template-imports/package.json'.ts(1479)
//
// ...Except,
// > the referenced file is an ECMAScript module
//
// package.json#exports does refer to a cjs file if required, so TS should be resolving the `require`
// entries not the `import` entries.
//
// https://github.com/embroider-build/content-tag/blob/v1.2.2-content-tag/package.json#L13-L21
//
// @ts-expect-error see above
import { Preprocessor } from 'content-tag';
const p = new Preprocessor();

export const preprocess: GlintExtensionPreprocess<PreprocessData> = (source, path) => {
let templates = parseTemplates(source, path, { templateTag: 'template' }).filter(
(match) => match.type === 'template-tag'
);
// NOTE: https://github.com/embroider-build/content-tag/issues/45
// All indicies are byte-index, not char-index.
let templates = p.parse(source, path);

let templateLocations: Array<TemplateLocation> = [];
let segments: Array<string> = [];
let sourceOffset = 0;
let delta = 0;
let sourceOffsetBytes = 0;
let deltaBytes = 0;

// @ts-expect-error TS couldn't find @types/node, which are specified
// in the root package.json
let sourceBuffer = Buffer.from(source);

for (let template of templates) {
let { start, end } = template;
let startTagLength = template.start[0].length;
let endTagLength = template.end[0].length;
let startTagOffset = start.index ?? -1;
let endTagOffset = end.index ?? -1;
let startTagLengthBytes = template.startRange.end - template.startRange.start;
let endTagLengthBytes = template.endRange.end - template.endRange.start;
let startTagOffsetBytes = template.startRange.start;
let endTagOffsetBytes = template.endRange.start;

if (startTagOffset === -1 || endTagOffset === -1) continue;
// if (startTagOffset === -1 || endTagOffset === -1) continue;

let transformedStart = startTagOffset - delta;
let transformedStartBytes = startTagOffsetBytes - deltaBytes;

segments.push(source.slice(sourceOffset, startTagOffset));
/**
* TODO: we want content-tag to manage all this for us, as managing indicies
* can be error-prone.
*
* SEE: https://github.com/embroider-build/content-tag/issues/39#issuecomment-1832443310
*/
let prefixingSegment = sourceBuffer.slice(sourceOffsetBytes, startTagOffsetBytes);
segments.push(prefixingSegment.toString());
segments.push(TEMPLATE_START);
delta += startTagLength - TEMPLATE_START.length;

let transformedEnd = endTagOffset - delta + TEMPLATE_END.length;
// For TEMPLATE_START & TEMPLATE_END, characters === bytes
deltaBytes += startTagLengthBytes - TEMPLATE_START.length;

let transformedEnd = endTagOffsetBytes - deltaBytes + TEMPLATE_END.length;

segments.push(source.slice(startTagOffset + startTagLength, endTagOffset));
let templateContentSegment = sourceBuffer.slice(
startTagOffsetBytes + startTagLengthBytes,
endTagOffsetBytes
);
segments.push(templateContentSegment.toString());
segments.push(TEMPLATE_END);
delta += endTagLength - TEMPLATE_END.length;
deltaBytes += endTagLengthBytes - TEMPLATE_END.length;

sourceOffset = endTagOffset + endTagLength;
sourceOffsetBytes = endTagOffsetBytes + endTagLengthBytes;

// TODO: is there a way to convert bytes to chars?
// I think maybe all of this code needs to live in content-tag,
// and give us the option to generate this sort of structure
templateLocations.push({
startTagOffset,
endTagOffset,
startTagLength,
endTagLength,
transformedStart,
startTagOffset: startTagOffsetBytes,
endTagOffset: endTagOffsetBytes,
startTagLength: startTagLengthBytes,
endTagLength: endTagLengthBytes,
transformedStart: transformedStartBytes,
transformedEnd,
});
}

segments.push(source.slice(sourceOffset));
segments.push(sourceBuffer.slice(sourceOffsetBytes).toString());

return {
contents: segments.join(''),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

export default interface Globals {
// used to hang any macros off of that are provided by config.additionalGlobals
}
}
3 changes: 1 addition & 2 deletions packages/environment-ember-template-imports/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"globals/index.d.ts"
],
"dependencies": {
"ember-template-imports": "^3.0.0"
"content-tag": "^1.2.2"
},
"peerDependencies": {
"@glint/environment-ember-loose": "^1.3.0",
Expand Down Expand Up @@ -57,7 +57,6 @@
"@types/ember__helper": "^4.0.1",
"@types/ember__modifier": "^4.0.3",
"@types/ember__routing": "^4.0.12",
"ember-template-imports": "^3.0.0",
"vitest": "^0.22.0"
},
"publishConfig": {
Expand Down
7 changes: 6 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5235,6 +5235,11 @@ [email protected]:
dependencies:
safe-buffer "5.2.1"

content-tag@^1.2.2:
version "1.2.2"
resolved "https://registry.yarnpkg.com/content-tag/-/content-tag-1.2.2.tgz#8cbc3cdb9957a81f7c425b138e334330dd6fd78d"
integrity sha512-9guqKIx2H+78N17otBpl8yLZbQGL5q1vBO/jDb3gF2JjixtcVpC62jDUNxjVMNoaZ09oxRX84ZOD6VX02qkVvg==

content-type@~1.0.4:
version "1.0.5"
resolved "https://registry.yarnpkg.com/content-type/-/content-type-1.0.5.tgz#8b773162656d1d1086784c8f23a54ce6d73d7918"
Expand Down Expand Up @@ -6545,7 +6550,7 @@ ember-template-imports@^1.1.1:
ember-cli-version-checker "^5.1.2"
validate-peer-dependencies "^1.1.0"

ember-template-imports@^3.0.0, ember-template-imports@^3.4.0:
ember-template-imports@^3.4.0:
version "3.4.2"
resolved "https://registry.yarnpkg.com/ember-template-imports/-/ember-template-imports-3.4.2.tgz#6cf7de7d4b8348a0fddf3aaec4947aa1211289e6"
integrity sha512-OS8TUVG2kQYYwP3netunLVfeijPoOKIs1SvPQRTNOQX4Pu8xGGBEZmrv0U1YTnQn12Eg+p6w/0UdGbUnITjyzw==
Expand Down

0 comments on commit 71b849c

Please sign in to comment.