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

Restyle error messages #2902

Merged
merged 3 commits into from
Mar 28, 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
5 changes: 5 additions & 0 deletions .changeset/small-radios-remain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Update CLI error format and style
6 changes: 3 additions & 3 deletions packages/astro/src/cli/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function offsetAt({ line, character }: { line: number; character: number }, text
return i;
}

function pad(str: string, len: number) {
function generateString(str: string, len: number) {
return Array.from({ length: len }, () => str).join('');
}

Expand Down Expand Up @@ -86,8 +86,8 @@ export async function check(astroConfig: AstroConfig) {
const lineNumStr = d.range.start.line.toString();
const lineNumLen = lineNumStr.length;
console.error(`${bgWhite(black(lineNumStr))} ${str}`);
let tildes = pad('~', d.range.end.character - d.range.start.character);
let spaces = pad(' ', d.range.start.character + lineNumLen - 1);
let tildes = generateString('~', d.range.end.character - d.range.start.character);
let spaces = generateString(' ', d.range.start.character + lineNumLen - 1);
console.error(` ${spaces}${bold(red(tildes))}\n`);
result.errors++;
break;
Expand Down
40 changes: 15 additions & 25 deletions packages/astro/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import add from '../core/add/index.js';
import devServer from '../core/dev/index.js';
import preview from '../core/preview/index.js';
import { check } from './check.js';
import { formatConfigError, loadConfig } from '../core/config.js';
import { printHelp } from '../core/messages.js';
import { loadConfig } from '../core/config.js';
import { printHelp, formatErrorMessage, formatConfigErrorMessage } from '../core/messages.js';
import { createSafeError } from '../core/util.js';

type Arguments = yargs.Arguments;
type CLICommand = 'help' | 'version' | 'add' | 'dev' | 'build' | 'preview' | 'reload' | 'check';
Expand Down Expand Up @@ -102,40 +103,33 @@ export async function cli(args: string[]) {
// For now, `add` has to resolve the config again internally
config = await loadConfig({ cwd: projectRoot, flags });
} catch (err) {
throwAndExit(err);
return;
return throwAndExit(err);
}

switch (cmd) {
case 'add': {
try {
const packages = flags._.slice(3) as string[];
await add(packages, { cwd: projectRoot, flags, logging });
process.exit(0);
return await add(packages, { cwd: projectRoot, flags, logging });
} catch (err) {
throwAndExit(err);
return throwAndExit(err);
}
return;
}
case 'dev': {
try {
await devServer(config, { logging });

await new Promise(() => {}); // don’t close dev server
return await new Promise(() => {}); // lives forever
} catch (err) {
throwAndExit(err);
return throwAndExit(err);
}
return;
}

case 'build': {
try {
await build(config, { logging });
process.exit(0);
return await build(config, { logging });
} catch (err) {
throwAndExit(err);
return throwAndExit(err);
}
return;
}

case 'check': {
Expand All @@ -145,11 +139,10 @@ export async function cli(args: string[]) {

case 'preview': {
try {
await preview(config, { logging }); // this will keep running
return await preview(config, { logging }); // this will keep running
} catch (err) {
throwAndExit(err);
return throwAndExit(err);
}
return;
}

default: {
Expand All @@ -159,14 +152,11 @@ export async function cli(args: string[]) {
}

/** Display error and exit */
function throwAndExit(err: any) {
function throwAndExit(err: unknown) {
if (err instanceof z.ZodError) {
console.error(formatConfigError(err));
} else if (err.stack) {
const [mainMsg, ...stackMsg] = err.stack.split('\n');
console.error(colors.red(mainMsg) + '\n' + colors.dim(stackMsg.join('\n')));
console.error(formatConfigErrorMessage(err));
} else {
console.error(colors.red(err.toString() || err));
console.error(formatErrorMessage(createSafeError(err)));
}
process.exit(1);
}
18 changes: 8 additions & 10 deletions packages/astro/src/core/add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export default async function add(names: string[], { cwd, flags, logging }: AddO
}
applyPolyfill();

// If no integrations were given, prompt the user for some popular ones.
if (names.length === 0) {
const response = await prompts([
{
Expand All @@ -72,16 +73,13 @@ export default async function add(names: string[], { cwd, flags, logging }: AddO
},
]);

if (!response.frameworks && !response.addons) {
info(logging, null, msg.cancelled(`Integrations skipped.`, `You can always run ${cyan('astro add')} later!`));
return;
}
const selected = [response.frameworks ?? [], response.addons ?? []].flat(1);
if (selected.length === 0) {
error(logging, null, `\n${red('No integrations specified!')}\n${dim('Try running')} astro add again.`);
return;
}
names = selected;
names = [...(response.frameworks ?? []), ...(response.addons ?? [])];
}

// If still empty after prompting, exit gracefully.
if (names.length === 0) {
error(logging, null, `No integrations specified.`);
return;
}

// Some packages might have a common alias! We normalize those here.
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/build/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { appendForwardSlash } from '../../core/path.js';

const STATUS_CODE_PAGES = new Set(['/404', '/500']);

export function getOutRoot(astroConfig: AstroConfig): URL {
function getOutRoot(astroConfig: AstroConfig): URL {
return new URL('./', astroConfig.dist);
}

Expand Down
159 changes: 72 additions & 87 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import fs from 'fs';
import { bgGreen, bgMagenta, black, cyan, dim, green, magenta } from 'kleur/colors';
import npath from 'path';
import type { OutputAsset, OutputChunk, RollupOutput } from 'rollup';
import { fileURLToPath } from 'url';
import type { AstroConfig, ComponentInstance, EndpointHandler, SSRLoadedRenderer } from '../../@types/astro';
import type { PageBuildData, StaticBuildOptions, SingleFileBuiltModule } from './types';
import type { BuildInternals } from '../../core/build/internal.js';
import { debug, info } from '../../core/logger.js';
import { appendForwardSlash, prependForwardSlash } from '../../core/path.js';
import type { RenderOptions } from '../../core/render/core';

import fs from 'fs';
import npath from 'path';
import { fileURLToPath } from 'url';
import { debug, error, info } from '../../core/logger.js';
import { prependForwardSlash } from '../../core/path.js';
import { BEFORE_HYDRATION_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
import { call as callEndpoint } from '../endpoint/index.js';
import { render } from '../render/core.js';
import { createLinkStylesheetElementSet, createModuleScriptElementWithSrcSet } from '../render/ssr-element.js';
import { getOutFile, getOutRoot, getOutFolder } from './common.js';
import { getPageDataByComponent, eachPageData } from './internal.js';
import { bgMagenta, black, cyan, dim, magenta } from 'kleur/colors';
import { getOutputFilename } from '../util.js';
import { getOutFile, getOutFolder } from './common.js';
import { eachPageData, getPageDataByComponent } from './internal.js';
import type { PageBuildData, SingleFileBuiltModule, StaticBuildOptions } from './types';
import { getTimeStat } from './util.js';

// Render is usually compute, which Node.js can't parallelize well.
Expand Down Expand Up @@ -67,7 +67,8 @@ export function chunkIsPage(astroConfig: AstroConfig, output: OutputAsset | Outp
}

export async function generatePages(result: RollupOutput, opts: StaticBuildOptions, internals: BuildInternals, facadeIdToPageDataMap: Map<string, PageBuildData>) {
info(opts.logging, null, `\n${bgMagenta(black(' generating static routes '))}\n`);
const timer = performance.now();
info(opts.logging, null, `\n${bgGreen(black(' generating static routes '))}`);

const ssr = !!opts.astroConfig._ctx.adapter?.serverEntrypoint;
const serverEntry = opts.buildConfig.serverEntry;
Expand All @@ -78,6 +79,7 @@ export async function generatePages(result: RollupOutput, opts: StaticBuildOptio
for (const pageData of eachPageData(internals)) {
await generatePage(opts, internals, pageData, ssrEntry);
}
info(opts.logging, null, dim(`Completed in ${getTimeStat(timer, performance.now())}.\n`));
}

async function generatePage(
Expand Down Expand Up @@ -109,31 +111,18 @@ async function generatePage(
renderers,
};

const icon = pageData.route.type === 'page' ? cyan('</>') : magenta('{-}');
const icon = pageData.route.type === 'page' ? green('▶') : magenta('λ');
info(opts.logging, null, `${icon} ${pageData.route.component}`);

// Throttle the paths to avoid overloading the CPU with too many tasks.
const renderPromises = [];
for (const paths of throttle(MAX_CONCURRENT_RENDERS, pageData.paths)) {
for (const path of paths) {
renderPromises.push(generatePath(path, opts, generationOptions));
}
// This blocks generating more paths until these 10 complete.
await Promise.all(renderPromises);
for (let i = 0; i < pageData.paths.length; i++) {
const path = pageData.paths[i];
await generatePath(path, opts, generationOptions);
const timeEnd = performance.now();
const timeChange = getTimeStat(timeStart, timeEnd);
let shouldLogTimeChange = !getTimeStat(timeStart, timeEnd).startsWith('0');
for (const path of paths) {
const timeIncrease = shouldLogTimeChange ? ` ${dim(`+${timeChange}`)}` : '';
info(opts.logging, null, ` ${dim('┃')} ${path}${timeIncrease}`);
// Should only log build time on the first generated path
// Logging for all generated paths adds extra noise
shouldLogTimeChange = false;
}
// Reset timeStart for the next batch of rendered paths
timeStart = performance.now();
// This empties the array without allocating a new one.
renderPromises.length = 0;
const timeIncrease = `(+${timeChange})`;
const filePath = getOutputFilename(opts.astroConfig, path);
const lineIcon = i === pageData.paths.length - 1 ? '└─' : '├─';
info(opts.logging, null, ` ${cyan(lineIcon)} ${dim(filePath)} ${dim(timeIncrease)}`);
}
}

Expand Down Expand Up @@ -175,65 +164,61 @@ async function generatePath(pathname: string, opts: StaticBuildOptions, gopts: G
}
}

try {
const options: RenderOptions = {
legacyBuild: false,
links,
logging,
markdownRender: astroConfig.markdownOptions.render,
mod,
origin,
pathname,
scripts,
renderers,
async resolve(specifier: string) {
const hashedFilePath = internals.entrySpecifierToBundleMap.get(specifier);
if (typeof hashedFilePath !== 'string') {
// If no "astro:scripts/before-hydration.js" script exists in the build,
// then we can assume that no before-hydration scripts are needed.
// Return this as placeholder, which will be ignored by the browser.
// TODO: In the future, we hope to run this entire script through Vite,
// removing the need to maintain our own custom Vite-mimic resolve logic.
if (specifier === BEFORE_HYDRATION_SCRIPT_ID) {
return 'data:text/javascript;charset=utf-8,//[no before-hydration script]';
}
throw new Error(`Cannot find the built path for ${specifier}`);
const options: RenderOptions = {
legacyBuild: false,
links,
logging,
markdownRender: astroConfig.markdownOptions.render,
mod,
origin,
pathname,
scripts,
renderers,
async resolve(specifier: string) {
const hashedFilePath = internals.entrySpecifierToBundleMap.get(specifier);
if (typeof hashedFilePath !== 'string') {
// If no "astro:scripts/before-hydration.js" script exists in the build,
// then we can assume that no before-hydration scripts are needed.
// Return this as placeholder, which will be ignored by the browser.
// TODO: In the future, we hope to run this entire script through Vite,
// removing the need to maintain our own custom Vite-mimic resolve logic.
if (specifier === BEFORE_HYDRATION_SCRIPT_ID) {
return 'data:text/javascript;charset=utf-8,//[no before-hydration script]';
}
const relPath = npath.posix.relative(pathname, '/' + hashedFilePath);
const fullyRelativePath = relPath[0] === '.' ? relPath : './' + relPath;
return fullyRelativePath;
},
method: 'GET',
headers: new Headers(),
route: pageData.route,
routeCache,
site: astroConfig.buildOptions.site,
ssr: opts.astroConfig.buildOptions.experimentalSsr,
};

let body: string;
if (pageData.route.type === 'endpoint') {
const result = await callEndpoint(mod as unknown as EndpointHandler, options);

if (result.type === 'response') {
throw new Error(`Returning a Response from an endpoint is not supported in SSG mode.`);
throw new Error(`Cannot find the built path for ${specifier}`);
}
body = result.body;
} else {
const result = await render(options);
const relPath = npath.posix.relative(pathname, '/' + hashedFilePath);
const fullyRelativePath = relPath[0] === '.' ? relPath : './' + relPath;
return fullyRelativePath;
},
method: 'GET',
headers: new Headers(),
route: pageData.route,
routeCache,
site: astroConfig.buildOptions.site,
ssr: opts.astroConfig.buildOptions.experimentalSsr,
};

// If there's a redirect or something, just do nothing.
if (result.type !== 'html') {
return;
}
body = result.html;
let body: string;
if (pageData.route.type === 'endpoint') {
const result = await callEndpoint(mod as unknown as EndpointHandler, options);

if (result.type === 'response') {
throw new Error(`Returning a Response from an endpoint is not supported in SSG mode.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh good catch!

Copy link
Member Author

Choose a reason for hiding this comment

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

I wish I came up with this! but it was just moved from elsewhere. You probably have @matthewp to thank :)

}
body = result.body;
} else {
const result = await render(options);

const outFolder = getOutFolder(astroConfig, pathname, pageData.route.type);
const outFile = getOutFile(astroConfig, outFolder, pathname, pageData.route.type);
await fs.promises.mkdir(outFolder, { recursive: true });
await fs.promises.writeFile(outFile, body, 'utf-8');
} catch (err) {
error(opts.logging, 'build', `Error rendering:`, err);
// If there's a redirect or something, just do nothing.
if (result.type !== 'html') {
return;
}
body = result.html;
}

const outFolder = getOutFolder(astroConfig, pathname, pageData.route.type);
const outFile = getOutFile(astroConfig, outFolder, pathname, pageData.route.type);
await fs.promises.mkdir(outFolder, { recursive: true });
await fs.promises.writeFile(outFile, body, 'utf-8');
}
Loading