Chore: fix lint rules, add more error handling for unexpected cases (#177)

.changeset/happy-chicken-whisper.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+chore: Remove non-null assertions

packages/astro-parser/src/parse/state/tag.ts

@@ -10,7 +10,6 @@ import { Directive, DirectiveType, TemplateNode, Text } from '../../interfaces.j
 import fuzzymatch from '../../utils/fuzzymatch.js';
 import list from '../../utils/list.js';
 
-// eslint-disable-next-line no-useless-escape
 const valid_tag_name = /^\!?[a-zA-Z]{1,}:?[a-zA-Z0-9\-]*/;
 
 const meta_tags = new Map([
@@ -393,7 +392,6 @@ function read_attribute(parser: Parser, unique_names: Set<string>) {
     }
   }
 
-  // eslint-disable-next-line no-useless-escape
   const name = parser.read_until(/[\s=\/>"']/);
   if (!name) return null;
 

packages/astro/src/ast.ts

@@ -20,10 +20,8 @@ export function getAttrValue(attributes: Attribute[], name: string): string | un
 /** Set TemplateNode attribute value */
 export function setAttrValue(attributes: Attribute[], name: string, value: string): void {
   const attr = attributes.find((a) => a.name === name);
-  if (attr) {
+  if (attr && attr.value[0]) {
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    attr.value[0].data = value;
-    attr.value[0]!.data = value;
+    attr.value[0].raw = value;
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    attr.value[0]!.raw = value;
   }
 }

packages/astro/src/build.ts

@@ -168,9 +168,8 @@ export async function build(astroConfig: AstroConfig): Promise<0 | 1> {
     info(logging, 'tip', `Set "buildOptions.site" in astro.config.mjs to generate a sitemap.xml, or set "buildOptions.sitemap: false" to disable this message.`);
   }
 
-  timer.write = performance.now();
-
   // write to disk and free up memory
+  timer.write = performance.now();
   await Promise.all(
     Object.keys(buildState).map(async (id) => {
       const outPath = new URL(`.${id}`, dist);
@@ -195,7 +194,7 @@ export async function build(astroConfig: AstroConfig): Promise<0 | 1> {
       publicFiles.map(async (filepath) => {
         const fileUrl = new URL(`file://${filepath}`);
         const rel = path.relative(fileURLToPath(pub), fileURLToPath(fileUrl));
-        const outPath = new URL(path.join('.', rel), dist);
+        const outPath = new URL('./' + rel, dist);
         await fs.promises.mkdir(path.dirname(fileURLToPath(outPath)), { recursive: true });
         await fs.promises.copyFile(fileUrl, outPath);
       })
@@ -209,7 +208,7 @@ export async function build(astroConfig: AstroConfig): Promise<0 | 1> {
   }
 
   /**
-   * 5. Bundling 2nd pass: On disk
+   * 5. Bundling 2nd Pass: On disk
    * Bundle JS, which requires hard files to optimize
    */
   info(logging, 'build', yellow(`! bundling...`));

packages/astro/src/build/bundle/js.ts

@@ -72,8 +72,9 @@ export async function bundleJS(imports: Set<string>, { runtime, dist }: BundleOp
     format: 'esm',
     exports: 'named',
     entryFileNames(chunk) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      const { facadeModuleId } = chunk;
-      return chunk.facadeModuleId!.substr(1);
+      if (!facadeModuleId) throw new Error(`facadeModuleId missing: ${chunk.name}`);
+      return facadeModuleId.substr(1);
     },
     plugins: [
       // We are using terser for the demo, but might switch to something else long term

packages/astro/src/build/page.ts

@@ -37,7 +37,7 @@ export function getPageType(filepath: URL): 'collection' | 'static' {
 
 /** Build collection */
 export async function buildCollectionPage({ astroConfig, filepath, logging, mode, runtime, site, resolvePackageUrl, buildState }: PageBuildOptions): Promise<void> {
-  const rel = path.relative(fileURLToPath(astroConfig.astroRoot) + '/pages', fileURLToPath(filepath)); // pages/index.astro
+  const rel = path.posix.relative(fileURLToPath(astroConfig.astroRoot) + '/pages', fileURLToPath(filepath)); // pages/index.astro
   const pagePath = `/${rel.replace(/\$([^.]+)\.astro$/, '$1')}`;
   const srcPath = new URL('pages/' + rel, astroConfig.astroRoot);
   const builtURLs = new Set<string>(); // !important: internal cache that prevents building the same URLs
@@ -102,7 +102,7 @@ export async function buildCollectionPage({ astroConfig, filepath, logging, mode
 
 /** Build static page */
 export async function buildStaticPage({ astroConfig, buildState, filepath, logging, mode, resolvePackageUrl, runtime }: PageBuildOptions): Promise<void> {
-  const rel = path.relative(fileURLToPath(astroConfig.astroRoot) + '/pages', fileURLToPath(filepath)); // pages/index.astro
+  const rel = path.posix.relative(fileURLToPath(astroConfig.astroRoot) + '/pages', fileURLToPath(filepath)); // pages/index.astro
   const pagePath = `/${rel.replace(/\.(astro|md)$/, '')}`;
 
   let relPath = path.posix.join('/', rel.replace(/\.(astro|md)$/, '.html'));
@@ -251,29 +251,31 @@ async function gatherRuntimes({ astroConfig, buildState, filepath, logging, reso
 
     switch (defn.plugin) {
       case 'preact': {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        const preact = dynamic.get('preact');
-        imports.add(dynamic.get('preact')!);
+        if (!preact) throw new Error(`Unable to load Preact plugin`);
+        imports.add(preact);
         rel = rel.replace(/\.[^.]+$/, '.js');
         break;
       }
       case 'react': {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        const [react, reactDOM] = [dynamic.get('react'), dynamic.get('react-dom')];
-        imports.add(dynamic.get('react')!);
+        if (!react || !reactDOM) throw new Error(`Unable to load React plugin`);
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        imports.add(react);
-        imports.add(dynamic.get('react-dom')!);
+        imports.add(reactDOM);
         rel = rel.replace(/\.[^.]+$/, '.js');
         break;
       }
       case 'vue': {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        const vue = dynamic.get('vue');
-        imports.add(dynamic.get('vue')!);
+        if (!vue) throw new Error('Unable to load Vue plugin');
+        imports.add(vue);
         rel = rel.replace(/\.[^.]+$/, '.vue.js');
         break;
       }
       case 'svelte': {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        const svelte = dynamic.get('svelte');
-        imports.add(dynamic.get('svelte')!);
+        if (!svelte) throw new Error('Unable to load Svelte plugin');
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        imports.add(svelte);
         imports.add('/_astro_internal/runtime/svelte.js');
         rel = rel.replace(/\.[^.]+$/, '.svelte.js');
         break;

packages/astro/src/cli.ts

@@ -120,8 +120,8 @@ export async function cli(args: string[]) {
     }
     case 'build':
     case 'dev': {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      const cmd = cmdMap.get(state.cmd);
-      const cmd = cmdMap.get(state.cmd)!;
+      if (!cmd) throw new Error(`Error running ${state.cmd}`);
       runCommand(flags._[3], cmd, state.options);
     }
   }

packages/astro/src/compiler/codegen/index.ts

@@ -423,21 +423,21 @@ function compileModule(module: Script, state: CodegenState, compileOptions: Comp
       if (plugin) {
         componentPlugins.add(plugin);
       }
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      const { start, end } = componentImport;
-      state.importExportStatements.add(module.content.slice(componentImport.start!, componentImport.end!));
+      state.importExportStatements.add(module.content.slice(start || undefined, end || undefined));
     }
     for (const componentImport of componentExports) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      const { start, end } = componentImport;
-      state.importExportStatements.add(module.content.slice(componentImport.start!, componentImport.end!));
+      state.importExportStatements.add(module.content.slice(start || undefined, end || undefined));
     }
 
     if (componentProps.length > 0) {
       propsStatement = 'let {';
       for (const componentExport of componentProps) {
         propsStatement += `${(componentExport.id as Identifier).name}`;
-        if (componentExport.init) {
+        const { init } = componentExport;
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        if (init) {
-          propsStatement += `= ${babelGenerator(componentExport.init!).code}`;
+          propsStatement += `= ${babelGenerator(init).code}`;
         }
         propsStatement += `,`;
       }
@@ -541,14 +541,12 @@ function compileHtml(enterNode: TemplateNode, state: CodegenState, compileOption
       switch (node.type) {
         case 'Expression': {
           let children: string[] = [];
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+          for (const child of node.children || []) {
-          for (const child of node.children!) {
             children.push(compileHtml(child, state, compileOptions));
           }
           let raw = '';
           let nextChildIndex = 0;
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+          for (const chunk of node.codeChunks) {
-          for (const chunk of node.codeChunks!) {
             raw += chunk;
             if (nextChildIndex < children.length) {
               raw += children[nextChildIndex++];

packages/astro/src/compiler/transform/doctype.ts

@@ -21,8 +21,7 @@ export default function (_opts: { filename: string; fileID: string }): Transform
                 name: '!doctype',
                 type: 'Element',
               };
-              // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+              (parent.children || []).splice(index, 0, dtNode);
-              parent.children!.splice(index, 0, dtNode);
               hasDoctype = true;
             }
           },

packages/astro/src/compiler/transform/index.ts

@@ -56,8 +56,7 @@ function walkAstWithVisitors(tmpl: TemplateNode, collection: VisitorCollection)
   walk(tmpl, {
     enter(node, parent, key, index) {
       if (collection.enter.has(node.type)) {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        const fns = collection.enter.get(node.type) || [];
-        const fns = collection.enter.get(node.type)!;
         for (let fn of fns) {
           fn.call(this, node, parent, key, index);
         }
@@ -65,8 +64,7 @@ function walkAstWithVisitors(tmpl: TemplateNode, collection: VisitorCollection)
     },
     leave(node, parent, key, index) {
       if (collection.leave.has(node.type)) {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        const fns = collection.leave.get(node.type) || [];
-        const fns = collection.leave.get(node.type)!;
         for (let fn of fns) {
           fn.call(this, node, parent, key, index);
         }

packages/astro/src/compiler/transform/styles.ts

@@ -1,3 +1,6 @@
+import type { TransformOptions, Transformer } from '../../@types/transformer';
+import type { TemplateNode } from 'astro-parser';
+
 import crypto from 'crypto';
 import fs from 'fs';
 import { createRequire } from 'module';
@@ -8,10 +11,7 @@ import postcss, { Plugin } from 'postcss';
 import postcssKeyframes from 'postcss-icss-keyframes';
 import findUp from 'find-up';
 import sass from 'sass';
-import type { RuntimeMode } from '../../@types/astro';
+import { debug, error, LogOptions } from '../../logger.js';
-import type { TransformOptions, Transformer } from '../../@types/transformer';
-import type { TemplateNode } from 'astro-parser';
-import { debug } from '../../logger.js';
 import astroScopedStyles, { NEVER_SCOPED_TAGS } from './postcss-scoped-styles/index.js';
 import slash from 'slash';
 
@@ -64,10 +64,10 @@ const miniCache: StylesMiniCache = {
 };
 
 export interface TransformStyleOptions {
+  logging: LogOptions;
   type?: string;
   filename: string;
   scopedClass: string;
-  mode: RuntimeMode;
 }
 
 /** given a class="" string, does it contain a given class? */
@@ -80,7 +80,7 @@ function hasClass(classList: string, className: string): boolean {
 }
 
 /** Convert styles to scoped CSS */
-async function transformStyle(code: string, { type, filename, scopedClass, mode }: TransformStyleOptions): Promise<StyleTransformResult> {
+async function transformStyle(code: string, { logging, type, filename, scopedClass }: TransformStyleOptions): Promise<StyleTransformResult> {
   let styleType: StyleType = 'css'; // important: assume CSS as default
   if (type) {
     styleType = getStyleType.get(type) || styleType;
@@ -128,8 +128,7 @@ async function transformStyle(code: string, { type, filename, scopedClass, mode
       const tw = require.resolve('tailwindcss', { paths: [import.meta.url, process.cwd()] });
       postcssPlugins.push(require(tw) as any);
     } catch (err) {
-      // eslint-disable-next-line no-console
+      error(logging, 'transform', err);
-      console.error(err);
       throw new Error(`tailwindcss not installed. Try running \`npm install tailwindcss\` and trying again.`);
     }
   }
@@ -192,10 +191,10 @@ export default function transformStyles({ compileOptions, filename, fileID }: Tr
               styleNodes.push(node);
               styleTransformPromises.push(
                 transformStyle(code, {
+                  logging: compileOptions.logging,
                   type: (langAttr && langAttr.value[0] && langAttr.value[0].data) || undefined,
                   filename,
                   scopedClass,
-                  mode: compileOptions.mode,
                 })
               );
               return;
@@ -246,10 +245,10 @@ export default function transformStyles({ compileOptions, filename, fileID }: Tr
             styleNodes.push(node);
             styleTransformPromises.push(
               transformStyle(code, {
+                logging: compileOptions.logging,
                 type: (langAttr && langAttr.value[0] && langAttr.value[0].data) || undefined,
                 filename,
                 scopedClass,
-                mode: compileOptions.mode,
               })
             );
           },

packages/astro/src/logger.ts

@@ -144,22 +144,21 @@ export const logger = {
   error: error.bind(null, defaultLogOptions),
 };
 
-// For silencing libraries that go directly to console.warn
+/** For silencing libraries that go directly to console.warn */
 export function trapWarn(cb: (...args: any[]) => void = () => {}) {
-  const warn = console.warn;
+  /* eslint-disable no-console */
+  const consoleWarn = console.warn;
   console.warn = function (...args: any[]) {
     cb(...args);
   };
-  return () => (console.warn = warn);
+  return () => (console.warn = consoleWarn);
 }
 
-
-
 function padStr(str: string, len: number) {
   const strLen = stringWidth(str);
-  if(strLen > len) {
+  if (strLen > len) {
     return str.substring(0, len - 3) + '...';
   }
   const spaces = Array.from({ length: len - strLen }, () => ' ').join('');
   return str + spaces;
-}
+}

packages/astro/test/.eslintrc.cjs

@@ -0,0 +1,5 @@
+module.exports = {
+  rules: {
+    'no-console': 'off',
+  },
+};

packages/astro/test/snowpack-integration.test.js

@@ -27,7 +27,6 @@ SnowpackDev.before(async () => {
   try {
     runtime = await createRuntime(astroConfig, { logging });
   } catch (err) {
-    // eslint-disable-next-line no-console
     console.error(err);
     setupError = err;
   }
@@ -82,7 +81,6 @@ SnowpackDev('Can load every page', async () => {
   }
 
   if (failed.length > 0) {
-    // eslint-disable-next-line no-console
     console.error(failed);
   }
   assert.equal(failed.length, 0, 'Failed pages');

tools/astro-languageserver/src/plugins/PluginHost.ts

@@ -4,7 +4,6 @@ import type * as d from './interfaces';
 import { flatten } from '../utils';
 import { FoldingRange } from 'vscode-languageserver-types';
 
-// eslint-disable-next-line no-shadow
 enum ExecuteMode {
   None,
   FirstNonNull,