Skip to content

Commit

Permalink
feat: improve rollup error reporting (#179)
Browse files Browse the repository at this point in the history
  • Loading branch information
manucorporat committed Feb 7, 2022
1 parent 379426f commit a92fcd8
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 50 deletions.
14 changes: 10 additions & 4 deletions src/optimizer/core/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use swc_ecmascript::visit::{FoldWith, VisitMutWith};
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct HookAnalysis {
pub origin: String,
pub origin: JsWord,
pub name: String,
pub entry: Option<JsWord>,
pub canonical_filename: String,
Expand Down Expand Up @@ -132,6 +132,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
JsWord::from(path_data.extension.clone())
};
let transpile = config.transpile;
let origin: JsWord = path_data.path.to_string_lossy().into();

match module {
Ok((main_module, comments, is_type_script, is_jsx)) => {
Expand Down Expand Up @@ -339,7 +340,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
},
);

let diagnostics = handle_error(&error_buffer, &source_map);
let diagnostics = handle_error(&error_buffer, origin, &source_map);
Ok(TransformOutput {
modules,
diagnostics,
Expand All @@ -353,7 +354,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
let error_buffer = ErrorBuffer::default();
let handler = Handler::with_emitter(true, false, Box::new(error_buffer.clone()));
err.into_diagnostic(&handler).emit();
let diagnostics = handle_error(&error_buffer, &source_map);
let diagnostics = handle_error(&error_buffer, origin, &source_map);
Ok(TransformOutput {
modules: vec![],
diagnostics,
Expand Down Expand Up @@ -456,7 +457,11 @@ pub fn emit_source_code(
}
}

fn handle_error(error_buffer: &ErrorBuffer, source_map: &Lrc<SourceMap>) -> Vec<Diagnostic> {
fn handle_error(
error_buffer: &ErrorBuffer,
origin: JsWord,
source_map: &Lrc<SourceMap>,
) -> Vec<Diagnostic> {
error_buffer
.0
.lock()
Expand Down Expand Up @@ -496,6 +501,7 @@ fn handle_error(error_buffer: &ErrorBuffer, source_map: &Lrc<SourceMap>) -> Vec<
};

Diagnostic {
origin: origin.clone(),
message,
code_highlights,
hints,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,4 @@ export const App_onmount = ()=>{
*/
== DIAGNOSTICS ==

[Diagnostic { message: "Identifier can not capture because it's a function: Thing", code_highlights: None, hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Identifier can not capture because it's a function: hola", code_highlights: None, hints: None, show_environment: false, severity: Error, documentation_url: None }]
[Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Identifier can not capture because it's a function: Thing", code_highlights: None, hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Identifier can not capture because it's a function: hola", code_highlights: None, hints: None, show_environment: false, severity: Error, documentation_url: None }]
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,4 @@ export const App_onmount = ()=>{
*/
== DIAGNOSTICS ==

[Diagnostic { message: "Identifier can not be captured", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 8, start_col: 16, end_line: 8, end_col: 20 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }]
[Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Identifier can not be captured", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 8, start_col: 16, end_line: 8, end_col: 20 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }]
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ export const App_onmount = ({ count })=>{
*/
== DIAGNOSTICS ==

[Diagnostic { message: "Reference to root level identifier needs to be exported: I10", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 7, start_col: 7, end_line: 7, end_col: 9 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I1", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 4, start_col: 7, end_line: 4, end_col: 8 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I2", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 8, end_line: 5, end_col: 9 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I3", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 13, end_line: 5, end_col: 14 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I4", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 22, end_line: 5, end_col: 23 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I5", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 27, end_line: 5, end_col: 28 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I6", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 37, end_line: 5, end_col: 38 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I7", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 42, end_line: 5, end_col: 43 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I8", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 52, end_line: 5, end_col: 53 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { message: "Reference to root level identifier needs to be exported: I9", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 6, start_col: 10, end_line: 6, end_col: 11 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }]
[Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I10", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 7, start_col: 7, end_line: 7, end_col: 9 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I1", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 4, start_col: 7, end_line: 4, end_col: 8 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I2", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 8, end_line: 5, end_col: 9 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I3", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 13, end_line: 5, end_col: 14 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I4", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 22, end_line: 5, end_col: 23 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I5", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 27, end_line: 5, end_col: 28 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I6", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 37, end_line: 5, end_col: 38 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I7", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 42, end_line: 5, end_col: 43 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I8", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 5, start_col: 52, end_line: 5, end_col: 53 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }, Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Reference to root level identifier needs to be exported: I9", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 6, start_col: 10, end_line: 6, end_col: 11 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }]
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,4 @@ export const App_onRender = ()=>{
*/
== DIAGNOSTICS ==

[Diagnostic { message: "Version without $ is not exported.", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 11, start_col: 5, end_line: 11, end_col: 12 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }]
[Diagnostic { origin: Atom('test.tsx' type=dynamic), message: "Version without $ is not exported.", code_highlights: Some([CodeHighlight { message: None, loc: SourceLocation { start_line: 11, start_col: 5, end_line: 11, end_col: 12 } }]), hints: None, show_environment: false, severity: Error, documentation_url: None }]
2 changes: 1 addition & 1 deletion src/optimizer/core/src/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub struct Hook {
pub local_idents: Vec<Id>,
pub scoped_idents: Vec<Id>,

pub origin: String,
pub origin: JsWord,
}

pub struct TransformContext {
Expand Down
2 changes: 2 additions & 0 deletions src/optimizer/core/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use serde::{Deserialize, Serialize};
use std::cmp::Ordering;
use swc_atoms::JsWord;

#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
pub struct SourceLocation {
Expand Down Expand Up @@ -42,6 +43,7 @@ pub struct CodeHighlight {

#[derive(Serialize, Deserialize, Debug)]
pub struct Diagnostic {
pub origin: JsWord,
pub message: String,
pub code_highlights: Option<Vec<CodeHighlight>>,
pub hints: Option<Vec<String>>,
Expand Down
26 changes: 25 additions & 1 deletion src/optimizer/src/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@

import type { NormalizedOutputOptions } from 'rollup';

// @alpha (undocumented)
export interface CodeHighlight {
// (undocumented)
loc: SourceLocation;
// (undocumented)
message: string | null;
}

// @alpha (undocumented)
export interface ComponentEntryStrategy {
// (undocumented)
Expand All @@ -17,20 +25,24 @@ export const createOptimizer: () => Promise<Optimizer>;

// @alpha (undocumented)
export interface Diagnostic {
// (undocumented)
code_highlights: CodeHighlight[];
// (undocumented)
documentation_url?: string;
// (undocumented)
hints?: string[];
// (undocumented)
message: string;
// (undocumented)
origin: string;
// (undocumented)
severity: DiagnosticType;
// (undocumented)
show_environment: boolean;
}

// @alpha (undocumented)
export type DiagnosticType = 'error' | 'warn' | 'info';
export type DiagnosticType = 'Error' | 'Warning' | 'SourceError';

// @alpha (undocumented)
export type EntryStrategy = SingleEntryStrategy | HookEntryStrategy | ComponentEntryStrategy | SmartEntryStrategy | ManualEntryStrategy;
Expand Down Expand Up @@ -166,6 +178,18 @@ export interface SmartEntryStrategy {
type: 'smart';
}

// @alpha (undocumented)
export interface SourceLocation {
// (undocumented)
end_col: number;
// (undocumented)
end_line: number;
// (undocumented)
start_col: number;
// (undocumented)
start_line: number;
}

// @alpha (undocumented)
export type SourceMapsOption = 'external' | 'inline' | undefined | null;

Expand Down
96 changes: 57 additions & 39 deletions src/optimizer/src/rollup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import {
TransformOutput,
TransformModule,
HookAnalysis,
Diagnostic,
} from '..';

import type { NormalizedOutputOptions } from 'rollup';
import type { NormalizedOutputOptions, PluginContext, RollupError } from 'rollup';
import type { Plugin } from 'vite';

/**
Expand All @@ -28,6 +29,33 @@ export function qwikRollup(opts: QwikPluginOptions): any {
...opts.entryStrategy,
};

const createRollupError = (rootDir: string, diagnostic: Diagnostic) => {
const loc = diagnostic.code_highlights[0]?.loc ?? {};
const id = optimizer.path.join(rootDir, diagnostic.origin);
const err: RollupError = Object.assign(new Error(diagnostic.message), {
id,
plugin: 'qwik',
loc: {
column: loc.start_col,
line: loc.start_line,
},
stack: '',
});
return err;
};

const handleDiagnostics = (ctx: PluginContext, rootDir: string, diagnostics: Diagnostic[]) => {
diagnostics.forEach((d) => {
if (d.severity === 'Error') {
ctx.error(createRollupError(rootDir, d));
} else if (d.severity === 'Warning') {
ctx.warn(createRollupError(rootDir, d));
} else {
ctx.warn(createRollupError(rootDir, d));
}
});
};

const plugin: Plugin = {
name: 'qwik',
enforce: 'pre',
Expand Down Expand Up @@ -63,36 +91,36 @@ export function qwikRollup(opts: QwikPluginOptions): any {

try {
const { render } = await server.ssrLoadModule('/src/entry.server.tsx');
const symbols = {
version: '1',
mapping: {} as Record<string, string>,
};
if (render) {
const symbols = {
version: '1',
mapping: {} as Record<string, string>,
};

Array.from(server.moduleGraph.fileToModulesMap.entries()).forEach((entry) => {
entry[1].forEach((v) => {
const hook = v.info?.meta?.hook;
if (hook && v.lastHMRTimestamp) {
symbols.mapping[hook.name] = `${v.url}?t=${v.lastHMRTimestamp}`;
}
Array.from(server.moduleGraph.fileToModulesMap.entries()).forEach((entry) => {
entry[1].forEach((v) => {
const hook = v.info?.meta?.hook;
if (hook && v.lastHMRTimestamp) {
symbols.mapping[hook.name] = `${v.url}?t=${v.lastHMRTimestamp}`;
}
});
});
const host = req.headers.host ?? 'localhost';
const result = await render({
url: new URL(`http://${host}${url}`),
debug: true,
symbols,
});
});
const host = req.headers.host ?? 'localhost';
const result = await render({
url: new URL(`http://${host}${url}`),
debug: true,
symbols,
});

const html = await server.transformIndexHtml(url, result.html);
res.setHeader('Content-Type', 'text/html; charset=utf-8');
res.writeHead(200);
res.end(html);
const html = await server.transformIndexHtml(url, result.html);
res.setHeader('Content-Type', 'text/html; charset=utf-8');
res.writeHead(200);
res.end(html);
}
} catch (e) {
server.ssrFixStacktrace(e as any);
// eslint-disable-next-line no-console
console.error(e as any);
res.writeHead(500);
res.end((e as any).message);
next(e);
}
} else {
next();
Expand Down Expand Up @@ -131,26 +159,14 @@ export function qwikRollup(opts: QwikPluginOptions): any {

const result = await optimizer.transformFs(transformOpts);
for (const output of result.modules) {
const key = optimizer.path.join(transformOpts.rootDir, output.path)!;
const key = optimizer.path.join(rootDir, output.path)!;
if (debug) {
// eslint-disable-next-line no-console
console.debug(`[QWIK PLUGIN] Module: ${key}`);
}
transformedOutputs.set(key, [output, key]);
}

// throw error or print logs if there are any diagnostics
result.diagnostics.forEach((d) => {
if (d.severity === 'error') {
throw d.message;
} else if (d.severity === 'warn') {
// eslint-disable-next-line no-console
console.warn('[QWIK PLUGIN]', d.message);
} else {
// eslint-disable-next-line no-console
console.info('[QWIK PLUGIN]', d.message);
}
});
handleDiagnostics(this, rootDir, result.diagnostics);

results.set('@buildStart', result);
}
Expand Down Expand Up @@ -245,6 +261,8 @@ export function qwikRollup(opts: QwikPluginOptions): any {
rootDir: dir,
});

handleDiagnostics(this, base, output.diagnostics);

if (output) {
results.set(id, output);
if (debug) {
Expand Down
22 changes: 21 additions & 1 deletion src/optimizer/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,10 @@ export interface TransformModule {
* @alpha
*/
export interface Diagnostic {
origin: string;
message: string;
severity: DiagnosticType;
code_highlights: CodeHighlight[];
documentation_url?: string;
show_environment: boolean;
hints?: string[];
Expand All @@ -131,7 +133,25 @@ export interface Diagnostic {
/**
* @alpha
*/
export type DiagnosticType = 'error' | 'warn' | 'info';
export interface CodeHighlight {
message: string | null;
loc: SourceLocation;
}

/**
* @alpha
*/
export interface SourceLocation {
start_line: number;
start_col: number;
end_line: number;
end_col: number;
}

/**
* @alpha
*/
export type DiagnosticType = 'Error' | 'Warning' | 'SourceError';

// ENTRY STRATEGY ***************

Expand Down

1 comment on commit a92fcd8

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: a92fcd8 Previous: 63a2bc7 Ratio
transform_todo_app 1713728 ns/iter (± 5089) 2312129 ns/iter (± 479200) 0.74

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.