refactor(client): fixed/hid type errors + extended isPoly (#58527)

client/src/templates/Challenges/classic/saved-challenges.test.ts

@@ -5,7 +5,6 @@ import type {
 import { mergeChallengeFiles } from './saved-challenges';
 
 const jsChallenge = {
-  id: '1',
   contents: 'js contents',
   fileKey: 'jsFileKey',
   name: 'name',
@@ -13,11 +12,11 @@ const jsChallenge = {
   head: 'head',
   tail: 'tail',
   history: [],
-  seed: 'original js contents'
+  seed: 'original js contents',
+  path: 'index.js'
 };
 
 const cssChallenge = {
-  id: '2',
   contents: 'css contents',
   fileKey: 'cssFileKey',
   name: 'name',
@@ -25,11 +24,11 @@ const cssChallenge = {
   head: 'head',
   tail: 'tail',
   history: [],
-  seed: 'original css contents'
+  seed: 'original css contents',
+  path: 'styles.css'
 };
 
 const htmlChallenge = {
-  id: '3',
   contents: 'html contents',
   fileKey: 'htmlFileKey',
   name: 'name',
@@ -37,7 +36,8 @@ const htmlChallenge = {
   head: 'head',
   tail: 'tail',
   history: [],
-  seed: 'original html contents'
+  seed: 'original html contents',
+  path: 'index.html'
 };
 
 const savedJsChallenge: SavedChallengeFile = {

client/src/templates/Challenges/utils/build.ts

@@ -51,7 +51,9 @@ const jsWorkerExecutor = new WorkerExecutor(jsTestEvaluator, {
   terminateWorker: true
 });
 
-type ApplyFunctionProps = (file: ChallengeFile) => Promise<ChallengeFile>;
+type ApplyFunctionProps = (
+  file: ChallengeFile
+) => Promise<ChallengeFile> | ChallengeFile;
 
 const applyFunction =
   (fn: ApplyFunctionProps) => async (file: ChallengeFile) => {
@@ -80,7 +82,7 @@ function buildSourceMap(challengeFiles: ChallengeFile[]): Source | undefined {
     (sources, challengeFile) => {
       sources.index += challengeFile.source || '';
       sources.contents = sources.index;
-      sources.original[challengeFile.history[0]] = challengeFile.source;
+      sources.original[challengeFile.history[0]] = challengeFile.source ?? null;
       sources.editableContents += challengeFile.editableContents || '';
       return sources;
     },
@@ -235,11 +237,11 @@ export async function buildDOMChallenge(
   );
   const isMultifile = challengeFiles.length > 1;
 
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-  const transformers =
-    isMultifile && hasJsx
-      ? getMultifileJSXTransformers(options)
-      : getTransformers(options);
+  // I'm reasonably sure this is fine, but we need to migrate transformers to
+  // TypeScript to be sure.
+  const transformers: ApplyFunctionProps[] = (isMultifile && hasJsx
+    ? getMultifileJSXTransformers(options)
+    : getTransformers(options)) as unknown as ApplyFunctionProps[];
 
   const pipeLine = composeFunctions(...transformers);
   const usesTestRunner = options?.usesTestRunner ?? false;
@@ -274,7 +276,9 @@ export async function buildJSChallenge(
   options: BuildOptions
 ): Promise<BuildResult> {
   if (!challengeFiles) throw Error('No challenge files provided');
-  const pipeLine = composeFunctions(...getTransformers(options));
+  const pipeLine = composeFunctions(
+    ...(getTransformers(options) as unknown as ApplyFunctionProps[])
+  );
 
   const finalFiles = await Promise.all(challengeFiles?.map(pipeLine));
   const error = finalFiles.find(({ error }) => error)?.error;
@@ -311,7 +315,9 @@ export async function buildPythonChallenge({
   challengeFiles
 }: BuildChallengeData): Promise<BuildResult> {
   if (!challengeFiles) throw new Error('No challenge files provided');
-  const pipeLine = composeFunctions(...getPythonTransformers());
+  const pipeLine = composeFunctions(
+    ...(getPythonTransformers() as unknown as ApplyFunctionProps[])
+  );
   const finalFiles = await Promise.all(challengeFiles.map(pipeLine));
   const error = finalFiles.find(({ error }) => error)?.error;
 

client/utils/__fixtures__/challenges.ts

@@ -2,7 +2,6 @@ import { ChallengeFile } from "../../src/redux/prop-types";
 
 export const challengeFiles: ChallengeFile[] = [
   {
-    id: '0',
     contents: 'some ts',
     error: null,
     ext: 'ts',
@@ -14,9 +13,9 @@ export const challengeFiles: ChallengeFile[] = [
     tail: '',
     editableRegionBoundaries: [],
     usesMultifileEditor: true,
+    path: 'index.ts',
   },
   {
-    id: '1',
     contents: 'some css',
     error: null,
     ext: 'css',
@@ -28,9 +27,9 @@ export const challengeFiles: ChallengeFile[] = [
     tail: '',
     editableRegionBoundaries: [],
     usesMultifileEditor: true,
+    path: 'styles.css',
   },
   {
-    id: '2',
     contents: 'some html',
     error: null,
     ext: 'html',
@@ -42,9 +41,9 @@ export const challengeFiles: ChallengeFile[] = [
     tail: '',
     editableRegionBoundaries: [],
     usesMultifileEditor: true,
+    path: 'index.html',
   },
   {
-    id: '3',
     contents: 'some js',
     error: null,
     ext: 'js',
@@ -56,9 +55,9 @@ export const challengeFiles: ChallengeFile[] = [
     tail: '',
     editableRegionBoundaries: [],
     usesMultifileEditor: true,
+    path: 'script.js',
   },
   {
-    id: '4',
     contents: 'some jsx',
     error: null,
     ext: 'jsx',
@@ -70,5 +69,6 @@ export const challengeFiles: ChallengeFile[] = [
     tail: '',
     editableRegionBoundaries: [],
     usesMultifileEditor: true,
+    path: 'index.jsx',
   }
 ]

shared/utils/polyvinyl.ts

@@ -1,12 +1,17 @@
 // originally based off of https://github.com/gulpjs/vinyl
 import invariant from 'invariant';
 
-export type Ext = 'js' | 'html' | 'css' | 'jsx' | 'ts';
+const exts = ['js', 'html', 'css', 'jsx', 'ts'] as const;
+export type Ext = (typeof exts)[number];
 
-export type ChallengeFile = {
+export type IncompleteChallengeFile = {
   fileKey: string;
   ext: Ext;
   name: string;
+  contents: string;
+};
+
+export type ChallengeFile = IncompleteChallengeFile & {
   editableRegionBoundaries?: number[];
   editableContents?: string;
   usesMultifileEditor?: boolean;
@@ -14,9 +19,8 @@ export type ChallengeFile = {
   head: string;
   tail: string;
   seed: string;
-  contents: string;
   source?: string | null;
-  id: string;
+  path: string;
   history: string[];
 };
 
@@ -65,25 +69,39 @@ export function createPoly<Rest>({
 }
 
 export function isPoly(poly: unknown): poly is ChallengeFile {
-  return (
-    !!poly &&
-    typeof poly === 'object' &&
-    'contents' in poly &&
+  function hasProperties(poly: unknown): poly is Record<string, unknown> {
+    return (
+      !!poly &&
+      typeof poly === 'object' &&
+      'contents' in poly &&
+      'name' in poly &&
+      'ext' in poly &&
+      'fileKey' in poly &&
+      'head' in poly &&
+      'tail' in poly &&
+      'seed' in poly &&
+      'history' in poly
+    );
+  }
+
+  const hasCorrectTypes = (poly: Record<string, unknown>): boolean =>
     typeof poly.contents === 'string' &&
-    'name' in poly &&
     typeof poly.name === 'string' &&
-    'ext' in poly &&
-    typeof poly.ext === 'string' &&
-    'history' in poly &&
-    Array.isArray(poly.history)
-  );
+    exts.includes(poly.ext as Ext) &&
+    typeof poly.fileKey === 'string' &&
+    typeof poly.head === 'string' &&
+    typeof poly.tail === 'string' &&
+    typeof poly.seed === 'string' &&
+    Array.isArray(poly.history);
+
+  return hasProperties(poly) && hasCorrectTypes(poly);
 }
 
 function checkPoly(poly: ChallengeFile) {
   invariant(
     isPoly(poly),
     'function should receive a PolyVinyl, but got %s',
-    poly
+    JSON.stringify(poly)
   );
 }
 
@@ -102,15 +120,14 @@ export function setContent(
 
 // This is currently only used to add back properties that are not stored in the
 // database.
-export function regeneratePathAndHistory(poly: ChallengeFile) {
-  const newPath = poly.name + '.' + poly.ext;
-  const newPoly = {
-    ...poly,
+export function regeneratePathAndHistory(file: IncompleteChallengeFile) {
+  const newPath = file.name + '.' + file.ext;
+  const newFile = {
+    ...file,
     path: newPath,
     history: [newPath]
   };
-  checkPoly(newPoly);
-  return newPoly;
+  return newFile;
 }
 
 async function clearHeadTail(polyP: Promise<ChallengeFile>) {