From 5da14ca8ed2ce4e88cdd04726f691e1c4ca69fab Mon Sep 17 00:00:00 2001 From: "Fred K. Schott" Date: Wed, 20 Oct 2021 22:09:35 -0700 Subject: [PATCH] comment style fixes (#1614) --- .eslintrc.cjs | 1 - packages/astro/src/@types/astro-core.ts | 1 + packages/astro/src/core/build/index.ts | 53 +++++++------------ packages/astro/src/core/config.ts | 6 +-- packages/astro/src/core/dev/index.ts | 37 +++++-------- packages/astro/src/core/logger.ts | 6 +-- packages/astro/src/core/ssr/index.ts | 42 +++++---------- packages/astro/src/core/util.ts | 4 -- packages/astro/src/runtime/server/index.ts | 22 +++----- .../vite-plugin-astro-postprocess/index.ts | 8 ++- packages/astro/src/vite-plugin-astro/index.ts | 6 ++- packages/astro/src/vite-plugin-fetch/index.ts | 12 +---- packages/astro/src/vite-plugin-jsx/index.ts | 22 +++----- .../astro/src/vite-plugin-markdown/index.ts | 6 +-- 14 files changed, 75 insertions(+), 151 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index d01fd35b3..0139256b4 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -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'], 'no-console': 'warn', 'no-shadow': 'error', 'prefer-const': 'off', diff --git a/packages/astro/src/@types/astro-core.ts b/packages/astro/src/@types/astro-core.ts index 826aadbbd..28c085cde 100644 --- a/packages/astro/src/@types/astro-core.ts +++ b/packages/astro/src/@types/astro-core.ts @@ -106,6 +106,7 @@ export interface AstroUserConfig { * export interface AstroUserConfig extends z.input { * } */ + export type AstroConfig = z.output; export type AsyncRendererComponentFn = (Component: any, props: any, children: string | undefined, metadata?: AstroComponentMetadata) => Promise; diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index a36abfe02..f13313b7d 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -51,16 +51,9 @@ class AstroBuilder { this.manifest = createRouteManifest({ config }); } - /** Build all pages */ async build() { - const timer: Record = {}; // 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 = {viteStart: performance.now()}; const viteConfig = await createVite( { mode: this.mode, @@ -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 = {}; // additional assets to be written + const assets: Record = {}; const allPages: Record = {}; - - // 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)]) { @@ -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]) => @@ -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', @@ -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; @@ -177,9 +162,10 @@ 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 }); @@ -187,10 +173,7 @@ class AstroBuilder { } 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 }); diff --git a/packages/astro/src/core/config.ts b/packages/astro/src/core/config.ts index aa74473f2..9657d36dc 100644 --- a/packages/astro/src/core/config.ts +++ b/packages/astro/src/core/config.ts @@ -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 { 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() diff --git a/packages/astro/src/core/dev/index.ts b/packages/astro/src/core/dev/index.ts index 25ab88ff4..71c14cbd8 100644 --- a/packages/astro/src/core/dev/index.ts +++ b/packages/astro/src/core/dev/index.ts @@ -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'; @@ -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((resolve, reject) => { const onError = (err: NodeJS.ErrnoException) => { if (err.code && err.code === 'EADDRINUSE') { @@ -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}` })); @@ -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 { if (!this.viteServer) throw new Error(`AstroDevServer.start() not called`); @@ -164,7 +158,6 @@ export class AstroDevServer { } } - /** Set up Vite server */ private async createViteServer() { const viteConfig = await createVite( { @@ -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 = {}; }); diff --git a/packages/astro/src/core/logger.ts b/packages/astro/src/core/logger.ts index a0a04b737..78a84eba7 100644 --- a/packages/astro/src/core/logger.ts +++ b/packages/astro/src/core/logger.ts @@ -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; diff --git a/packages/astro/src/core/ssr/index.ts b/packages/astro/src/core/ssr/index.ts index 02fb1c423..5626b88c9 100644 --- a/packages/astro/src/core/ssr/index.ts +++ b/packages/astro/src/core/ssr/index.ts @@ -38,11 +38,9 @@ const cache = new Map>(); // 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); @@ -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 { 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) { @@ -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(), @@ -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, @@ -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); @@ -183,7 +165,7 @@ ${frame} throw err; } - // Vite error (already formatted) + // Generic error (probably from Vite, and already formatted) throw e; } } diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index 1e696a4ec..eeb061ae1 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -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) { diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index 27eb5b5d2..c5654587c 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -10,18 +10,14 @@ export { createMetadata } from './metadata.js'; const { generate, GENERATOR } = astring; -/* - * A more robust version alternative to `JSON.stringify` that can handle most values - * See https://github.com/remcohaszing/estree-util-value-to-estree#readme - */ +// A more robust version alternative to `JSON.stringify` that can handle most values +// see https://github.com/remcohaszing/estree-util-value-to-estree#readme const customGenerator: astring.Generator = { ...GENERATOR, Literal(node, state) { if (node.raw != null) { - /* - * escape closing script tags in strings so browsers wouldn't interpret them as - * closing the actual end tag in HTML - */ + // escape closing script tags in strings so browsers wouldn't interpret them as + // closing the actual end tag in HTML state.write(node.raw.replace('', '<\\/script>')); } else { GENERATOR.Literal(node, state); @@ -39,11 +35,9 @@ async function _render(child: any): Promise { if (Array.isArray(child)) { return (await Promise.all(child.map((value) => _render(value)))).join('\n'); } else if (typeof child === 'function') { - /* - * Special: If a child is a function, call it automatically. - * This lets you do {() => ...} without the extra boilerplate - * of wrapping it in a function and calling it. - */ + // Special: If a child is a function, call it automatically. + // This lets you do {() => ...} without the extra boilerplate + // of wrapping it in a function and calling it. return _render(child()); } else if (typeof child === 'string') { return child; @@ -267,7 +261,7 @@ function createFetchContentFn(url: URL) { ...mod.frontmatter, content: mod.metadata, file: new URL(spec, url), - url: urlSpec.includes('/pages/') && urlSpec.replace(/^.*\/pages\//, '/').replace(/\.md$/, ''), + url: urlSpec.includes('/pages/') && urlSpec.replace(/^.*\/pages\//, '/').replace(/\.md$/, '') }; }) .filter(Boolean); diff --git a/packages/astro/src/vite-plugin-astro-postprocess/index.ts b/packages/astro/src/vite-plugin-astro-postprocess/index.ts index 20cda778d..c4f4f1ab5 100644 --- a/packages/astro/src/vite-plugin-astro-postprocess/index.ts +++ b/packages/astro/src/vite-plugin-astro-postprocess/index.ts @@ -11,7 +11,7 @@ interface AstroPluginOptions { devServer?: AstroDevServer; } -// esbuild transforms the component-scoped Astro into Astro2, so need to check both. +// esbuild transforms the component-scoped Astro into Astro2, so need to check both. const validAstroGlobalNames = new Set(['Astro', 'Astro2']); export default function astro({ config, devServer }: AstroPluginOptions): Plugin { @@ -23,10 +23,8 @@ export default function astro({ config, devServer }: AstroPluginOptions): Plugin return null; } - /* - * Optimization: only run on a probably match - * Open this up if need for post-pass extends past fetchContent - */ + // Optimization: only run on a probably match + // Open this up if need for post-pass extends past fetchContent if (!code.includes('fetchContent')) { return null; } diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index 4d69bcfaa..f06e0ad39 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -28,14 +28,16 @@ export default function astro({ config, devServer }: AstroPluginOptions): Plugin let tsResult: TransformResult | undefined; try { - // `.astro` -> `.ts` + // Transform from `.astro` to valid `.ts` + // use `sourcemap: "both"` so that sourcemap is included in the code + // result passed to esbuild, but also available in the catch handler. tsResult = await transform(source, { site: config.buildOptions.site, sourcefile: id, sourcemap: 'both', internalURL: 'astro/internal', }); - // `.ts` -> `.js` + // Compile `.ts` to `.js` const { code, map } = await esbuild.transform(tsResult.code, { loader: 'ts', sourcemap: 'external', sourcefile: id }); return { diff --git a/packages/astro/src/vite-plugin-fetch/index.ts b/packages/astro/src/vite-plugin-fetch/index.ts index b8af71d4e..58936e79d 100644 --- a/packages/astro/src/vite-plugin-fetch/index.ts +++ b/packages/astro/src/vite-plugin-fetch/index.ts @@ -15,10 +15,8 @@ function isSSR(options: undefined | boolean | { ssr: boolean }): boolean { return false; } -/* - * This matches any JS-like file (that we know of) - * See https://regex101.com/r/Cgofir/1 - */ +// This matches any JS-like file (that we know of) +// See https://regex101.com/r/Cgofir/1 const SUPPORTED_FILES = /\.(astro|svelte|vue|[cm]?js|jsx|[cm]?ts|tsx)$/; const DEFINE_FETCH = `import fetch from 'node-fetch';\n`; @@ -28,17 +26,14 @@ export default function pluginFetch(): Plugin { enforce: 'post', async transform(code, id, opts) { const ssr = isSSR(opts); - // If this isn't an SSR pass, `fetch` will already be available! if (!ssr) { return null; } - // Only transform JS-like files if (!id.match(SUPPORTED_FILES)) { return null; } - // Optimization: only run on probable matches if (!code.includes('fetch')) { return null; @@ -46,14 +41,11 @@ export default function pluginFetch(): Plugin { const s = new MagicString(code); s.prepend(DEFINE_FETCH); - const result = s.toString(); - const map = s.generateMap({ source: id, includeContent: true, }); - return { code: result, map }; }, }; diff --git a/packages/astro/src/vite-plugin-jsx/index.ts b/packages/astro/src/vite-plugin-jsx/index.ts index 4c4467c7c..4810c5009 100644 --- a/packages/astro/src/vite-plugin-jsx/index.ts +++ b/packages/astro/src/vite-plugin-jsx/index.ts @@ -18,10 +18,8 @@ const IMPORT_STATEMENTS: Record = { preact: "import { h } from 'preact'", 'solid-js': "import 'solid-js/web'", }; -/* - * The `tsx` loader in esbuild will remove unused imports, so we need to - * be careful about esbuild not treating h, React, Fragment, etc. as unused. - */ +// The `tsx` loader in esbuild will remove unused imports, so we need to +// be careful about esbuild not treating h, React, Fragment, etc. as unused. const PREVENT_UNUSED_IMPORTS = ';;(React,Fragment,h);'; interface AstroPluginJSXOptions { @@ -55,23 +53,15 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin } } - /* - * Single JSX renderer - * If we only have one renderer, we can skip a bunch of work! - */ + // Attempt: Single JSX renderer + // If we only have one renderer, we can skip a bunch of work! if (JSX_RENDERERS.size === 1) { return transformJSX({ code, id, renderer: [...JSX_RENDERERS.values()][0], ssr: ssr || false }); } - /* - * Multiple JSX renderers - * Determine for each .jsx or .tsx file what it wants to use to Render - */ - + // Attempt: Multiple JSX renderers + // Determine for each .jsx or .tsx file what it wants to use to Render // we need valid JS here, so we can use `h` and `Fragment` as placeholders - - // try and guess renderer from imports (file can’t import React and Preact) - // NOTE(fks, matthewp): Make sure that you're transforming the original contents here. const { code: codeToScan } = await esbuild.transform(code + PREVENT_UNUSED_IMPORTS, { loader: getLoader(path.extname(id)), diff --git a/packages/astro/src/vite-plugin-markdown/index.ts b/packages/astro/src/vite-plugin-markdown/index.ts index 19cbf3c12..9d34bb1bd 100644 --- a/packages/astro/src/vite-plugin-markdown/index.ts +++ b/packages/astro/src/vite-plugin-markdown/index.ts @@ -20,7 +20,7 @@ export default function markdown({ config }: AstroPluginOptions): Plugin { if (id.endsWith('.md')) { let source = await fs.promises.readFile(id, 'utf8'); - // `.md` -> `.astro` + // Transform from `.md` to valid `.astro` let render = config.markdownOptions.render; let renderOpts = {}; if (Array.isArray(render)) { @@ -46,14 +46,14 @@ ${setup} astroResult = `${prelude}\n${astroResult}`; } - // `.astro` -> `.ts` + // Transform from `.astro` to valid `.ts` let { code: tsResult } = await transform(astroResult, { sourcefile: id, sourcemap: 'inline', internalURL: 'astro/internal' }); tsResult = `\nexport const metadata = ${JSON.stringify(metadata)}; export const frontmatter = ${JSON.stringify(content)}; ${tsResult}`; - // `.ts` -> `.js` + // Compile from `.ts` to `.js` const { code, map } = await esbuild.transform(tsResult, { loader: 'ts', sourcemap: 'inline', sourcefile: id }); return {