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

More comment style changes #1614

Merged
merged 1 commit into from
Oct 21, 2021
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
1 change: 0 additions & 1 deletion .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ module.exports = {
'@typescript-eslint/no-unused-vars': 'off',
'@typescript-eslint/no-use-before-define': 'off',
'@typescript-eslint/no-var-requires': 'off',
'multiline-comment-style': ['warn', 'starred-block'],
Copy link
Member

Choose a reason for hiding this comment

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

(also posted in Discord) My only suggestion for this PR is if we want to discourage multi-line block comments, there’s a lint rule for that!™

'multiline-comment-style': ['warn', 'separate-lines']

If we want to suggest a certain style, let’s suggest it! A warning is friendly.

'no-console': 'warn',
'no-shadow': 'error',
'prefer-const': 'off',
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/@types/astro-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export interface AstroUserConfig {
* export interface AstroUserConfig extends z.input<typeof AstroConfigSchema> {
* }
*/

export type AstroConfig = z.output<typeof AstroConfigSchema>;

export type AsyncRendererComponentFn<U> = (Component: any, props: any, children: string | undefined, metadata?: AstroComponentMetadata) => Promise<U>;
Expand Down
53 changes: 18 additions & 35 deletions packages/astro/src/core/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,9 @@ class AstroBuilder {
this.manifest = createRouteManifest({ config });
}

/** Build all pages */
async build() {
const timer: Record<string, number> = {}; // keep track of performance timers

/*
* Setup
* Create the Vite server with production build settings
*/
timer.viteStart = performance.now();
const { logging, origin } = this;
const timer: Record<string, number> = {viteStart: performance.now()};
const viteConfig = await createVite(
{
mode: this.mode,
Expand All @@ -76,25 +69,22 @@ class AstroBuilder {
this.viteServer = viteServer;
debug(logging, 'build', timerMessage('Vite started', timer.viteStart));

/*
* Render pages
* Convert .astro -> .html
*/
timer.renderStart = performance.now();
const assets: Record<string, string> = {}; // additional assets to be written
const assets: Record<string, string> = {};
const allPages: Record<string, RouteData & { paths: string[] }> = {};

// pre-render: determine all possible routes from dynamic pages
// Collect all routes ahead-of-time, before we start the build.
// NOTE: This enforces that `getStaticPaths()` is only called once per route,
// and is then cached across all future SSR builds. In the past, we've had trouble
// with parallelized builds without guaranteeing that this is called first.
await Promise.all(
this.manifest.routes.map(async (route) => {
// static route
// static route:
if (route.pathname) {
allPages[route.component] = { ...route, paths: [route.pathname] };
return;
}
// dynamic route
// dynamic route:
const result = await this.getStaticPathsForRoute(route);
// handle RSS while generating routes
if (result.rss?.xml) {
const rssFile = new URL(result.rss.url.replace(/^\/?/, './'), this.config.dist);
if (assets[fileURLToPath(rssFile)]) {
Expand All @@ -106,7 +96,10 @@ class AstroBuilder {
})
);

// render: convert Astro to HTML
// After all routes have been collected, start building them.
// TODO: test parallel vs. serial performance. Promise.all() may be
// making debugging harder without any perf gain. If parallel is best,
// then we should set a max number of parallel builds.
const input: InputHTMLOptions[] = [];
await Promise.all(
Object.entries(allPages).map(([component, route]) =>
Expand All @@ -132,10 +125,8 @@ class AstroBuilder {
);
debug(logging, 'build', timerMessage('All pages rendered', timer.renderStart));

/*
* Production Build
* Use Vite’s build process to generate files
*/
// Bundle the assets in your final build: This currently takes the HTML output
// of every page (stored in memory) and bundles the assets pointed to on those pages.
timer.buildStart = performance.now();
await vite.build({
logLevel: 'error',
Expand All @@ -160,13 +151,7 @@ class AstroBuilder {
});
debug(logging, 'build', timerMessage('Vite build finished', timer.buildStart));

/*
* Post-build files
* Write other files to disk Vite may not know about.
* TODO: is there a way to handle these as part of the previous step?
*/

// RSS
// Write any additionally generated assets to disk.
timer.assetsStart = performance.now();
Object.keys(assets).map((k) => {
if (!assets[k]) return;
Expand All @@ -177,20 +162,18 @@ class AstroBuilder {
});
debug(logging, 'build', timerMessage('Additional assets copied', timer.assetsStart));

// Sitemap
// Build your final sitemap.
timer.sitemapStart = performance.now();
if (this.config.buildOptions.sitemap && this.config.buildOptions.site) {
const sitemapStart = performance.now();
const sitemap = generateSitemap(input.map(({ name }) => new URL(`/${name}`, this.config.buildOptions.site).href));
const sitemapPath = new URL('./sitemap.xml', this.config.dist);
await fs.promises.mkdir(new URL('./', sitemapPath), { recursive: true });
await fs.promises.writeFile(sitemapPath, sitemap, 'utf8');
}
debug(logging, 'build', timerMessage('Sitemap built', timer.sitemapStart));

/*
* Clean up
* Close the Vite server instance, and print stats
*/
// You're done! Time to clean up.
await viteServer.close();
if (logging.level && levels[logging.level] <= levels['info']) {
await this.printStats({ cwd: this.config.dist, pageCount: input.length });
Expand Down
6 changes: 2 additions & 4 deletions packages/astro/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ export const AstroConfigSchema = z.object({
/** Turn raw config values into normalized values */
export async function validateConfig(userConfig: any, root: string): Promise<AstroConfig> {
const fileProtocolRoot = pathToFileURL(root + path.sep);
/*
* We need to extend the global schema to add transforms that are relative to root.
* This is type checked against the global schema to make sure we still match.
*/
// We need to extend the global schema to add transforms that are relative to root.
// This is type checked against the global schema to make sure we still match.
const AstroConfigRelativeSchema = AstroConfigSchema.extend({
projectRoot: z
.string()
Expand Down
37 changes: 14 additions & 23 deletions packages/astro/src/core/dev/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { LogOptions } from '../logger';
import type { HmrContext, ModuleNode } from '../vite';

import { fileURLToPath } from 'url';
import {promisify} from 'util';
import connect from 'connect';
import mime from 'mime';
import { performance } from 'perf_hooks';
Expand Down Expand Up @@ -68,24 +69,16 @@ export class AstroDevServer {
this.manifest = createRouteManifest({ config });
}

/** Start dev server */
async start() {
/*
* Setup
* Create the Vite serer in dev mode
*/
const devStart = performance.now(); // profile startup time
this.viteServer = await this.createViteServer();
const devStart = performance.now();

// middlewares
// Setup the dev server and connect it to Vite (via middleware)
this.viteServer = await this.createViteServer();
this.app.use((req, res, next) => this.handleRequest(req, res, next));
this.app.use(this.viteServer.middlewares);
this.app.use((req, res, next) => this.renderError(req, res, next));

/*
* Listen
* Start external connect server and listen on configured port
*/
// Listen on port (and retry if taken)
await new Promise<void>((resolve, reject) => {
const onError = (err: NodeJS.ErrnoException) => {
if (err.code && err.code === 'EADDRINUSE') {
Expand All @@ -97,7 +90,6 @@ export class AstroDevServer {
reject(err);
}
};

this.httpServer = this.app.listen(this.port, this.hostname, () => {
info(this.logging, 'astro', msg.devStart({ startupTime: performance.now() - devStart }));
info(this.logging, 'astro', msg.devHost({ host: `http://${this.hostname}:${this.port}` }));
Expand All @@ -107,13 +99,15 @@ export class AstroDevServer {
});
}

/** Stop dev server */
async stop() {
this.httpServer?.close(); // close HTTP server
if (this.viteServer) await this.viteServer.close(); // close Vite server
if (this.viteServer) {
await this.viteServer.close();
}
if (this.httpServer) {
await promisify(this.httpServer.close)();
}
}

/** Handle HMR */
public async handleHotUpdate({ file, modules }: HmrContext): Promise<void | ModuleNode[]> {
if (!this.viteServer) throw new Error(`AstroDevServer.start() not called`);

Expand Down Expand Up @@ -164,7 +158,6 @@ export class AstroDevServer {
}
}

/** Set up Vite server */
private async createViteServer() {
const viteConfig = await createVite(
{
Expand Down Expand Up @@ -197,11 +190,9 @@ export class AstroDevServer {
this.manifest = createRouteManifest({ config: this.config });
});
viteServer.watcher.on('change', () => {
/*
* No need to rebuild routes on file content changes.
* However, we DO want to clear the cache in case
* the change caused a getStaticPaths() return to change.
*/
// No need to rebuild routes on file content changes.
// However, we DO want to clear the cache in case
// the change caused a getStaticPaths() return to change.
this.routeCache = {};
});

Expand Down
6 changes: 2 additions & 4 deletions packages/astro/src/core/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ function getLoggerLocale(): string {
const defaultLocale = 'en-US';
if (process.env.LANG) {
const extractedLocale = process.env.LANG.split('.')[0].replace(/_/g, '-');
/*
* Check if language code is at least two characters long (ie. en, es).
* NOTE: if "c" locale is encountered, the default locale will be returned.
*/
// Check if language code is atleast two characters long (ie. en, es).
// NOTE: if "c" locale is encountered, the default locale will be returned.
if (extractedLocale.length < 2) return defaultLocale;
else return extractedLocale;
} else return defaultLocale;
Expand Down
42 changes: 12 additions & 30 deletions packages/astro/src/core/ssr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,9 @@ const cache = new Map<string, Promise<Renderer>>();
// TODO: improve validation and error handling here.
async function resolveRenderer(viteServer: ViteDevServer, renderer: string) {
const resolvedRenderer: any = {};
/*
* We can dynamically import the renderer by itself because it shouldn't have
* any non-standard imports, the index is just meta info.
* The other entrypoints need to be loaded through Vite.
*/
// We can dynamically import the renderer by itself because it shouldn't have
// any non-standard imports, the index is just meta info.
// The other entrypoints need to be loaded through Vite.
const {
default: { name, client, polyfills, hydrationPolyfills, server },
} = await import(renderer);
Expand Down Expand Up @@ -75,20 +73,11 @@ async function resolveRenderers(viteServer: ViteDevServer, ids: string[]): Promi
/** use Vite to SSR */
export async function ssr({ astroConfig, filePath, logging, mode, origin, pathname, route, routeCache, viteServer }: SSROptions): Promise<string> {
try {
/*
* Renderers
* Load all renderers to be used for SSR
*/
// Important this happens before load module in case a renderer provides polyfills.
// Important: This needs to happen first, in case a renderer provides polyfills.
const renderers = await resolveRenderers(viteServer, astroConfig.renderers);

/*
* Pre-render
* Load module through Vite and do pre-render work like dynamic routing
*/
// Load the module from the Vite SSR Runtime.
const mod = (await viteServer.ssrLoadModule(fileURLToPath(filePath))) as ComponentInstance;

// handle dynamic routes
// Handle dynamic routes
let params: Params = {};
let pageProps: Props = {};
if (route && !route.pathname) {
Expand Down Expand Up @@ -118,15 +107,14 @@ export async function ssr({ astroConfig, filePath, logging, mode, origin, pathna
pageProps = { ...matchedStaticPath.props } || {};
}

/*
* Render
* Convert .astro to .html
*/
// Validate the page component before rendering the page
const Component = await mod.default;
if (!Component) throw new Error(`Expected an exported Astro component but received typeof ${typeof Component}`);

if (!Component.isAstroComponentFactory) throw new Error(`Unable to SSR non-Astro component (${route?.component})`);

// Create the result object that will be passed into the render function.
// This object starts here as an empty shell (not yet the result) but then
// calling the render() function will populate the object with scripts, styles, etc.
const result: SSRResult = {
styles: new Set(),
scripts: new Set(),
Expand All @@ -135,7 +123,6 @@ export async function ssr({ astroConfig, filePath, logging, mode, origin, pathna
const site = new URL(origin);
const url = new URL('.' + pathname, site);
const canonicalURL = getCanonicalURL('.' + pathname, astroConfig.buildOptions.site || origin);

return {
__proto__: astroGlobal,
props,
Expand All @@ -154,15 +141,10 @@ export async function ssr({ astroConfig, filePath, logging, mode, origin, pathna
};

let html = await renderPage(result, Component, pageProps, null);

/*
* Dev Server
* Apply Vite HMR, Astro HMR, and other dev-only transformations needed from Vite plugins.
*/
// run transformIndexHtml() in development to add HMR client to the page.
if (mode === 'development') {
html = await viteServer.transformIndexHtml(fileURLToPath(filePath), html, pathname);
}

return html;
} catch (e: any) {
viteServer.ssrFixStacktrace(e);
Expand All @@ -183,7 +165,7 @@ ${frame}
throw err;
}

// Vite error (already formatted)
// Generic error (probably from Vite, and already formatted)
throw e;
}
}
4 changes: 0 additions & 4 deletions packages/astro/src/core/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,18 @@ export function parseNpmName(spec: string): { scope?: string; name: string; subp
/** generate code frame from esbuild error */
export function codeFrame(src: string, loc: ErrorPayload['err']['loc']): string {
if (!loc) return '';

const lines = src.replace(/\r\n/g, '\n').split('\n');

// grab 2 lines before, and 3 lines after focused line
const visibleLines = [];
for (let n = -2; n <= 2; n++) {
if (lines[loc.line + n]) visibleLines.push(loc.line + n);
}

// figure out gutter width
let gutterWidth = 0;
for (const lineNo of visibleLines) {
let w = `> ${lineNo}`;
if (w.length > gutterWidth) gutterWidth = w.length;
}

// print lines
let output = '';
for (const lineNo of visibleLines) {
Expand Down
Loading