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

feat: improve rollup error reporting #179

Merged
merged 2 commits into from
Feb 7, 2022
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
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