fixup! big refactor

Author: jsjoeio

src/browser/pages/vscode.ts

@@ -54,16 +54,13 @@ export function getNlsConfiguration(document: Document) {
   return JSON.parse(nlsConfig) as NlsConfiguration
 }
 
-type RegisterRequireOnSelfType = {
-  // NOTE@jsjoeio
-  // We get the self type by looking at window.self.
-  self: Window & typeof globalThis
+type GetLoaderParams = {
   origin: string
   nlsConfig: NlsConfiguration
   options: Options
 }
 
-type RequireOnSelfType = {
+type Loader = {
   baseUrl: string
   recordStats: boolean
   paths: {
@@ -73,36 +70,30 @@ type RequireOnSelfType = {
 }
 
 /**
- * A helper function to register the require on self.
+ * A helper function to get the require loader
  *
- * The require property is used by VSCode/code-server
+ * This used by VSCode/code-server
  * to load files.
  *
  * We extracted the logic into a function so that
  * it's easier to test.
  **/
-export function registerRequireOnSelf({ self, origin, nlsConfig, options }: RegisterRequireOnSelfType) {
+export function getLoader({ origin, nlsConfig, options }: GetLoaderParams) {
   const errorMsgPrefix = "[vscode]"
 
-  if (!self) {
-    throw new Error(`${errorMsgPrefix} Could not register require on self. self is undefined.`)
-  }
-
   if (!origin) {
-    throw new Error(`${errorMsgPrefix} Could not register require on self. origin is undefined or missing.`)
+    throw new Error(`${errorMsgPrefix} Could not get loader. origin is undefined or missing.`)
   }
 
   if (!options || !options.csStaticBase) {
-    throw new Error(
-      `${errorMsgPrefix} Could not register require on self. options or options.csStaticBase is undefined or missing.`,
-    )
+    throw new Error(`${errorMsgPrefix} Could not get loader. options or options.csStaticBase is undefined or missing.`)
   }
 
   if (!nlsConfig) {
-    throw new Error(`${errorMsgPrefix} Could not register require on self. nlsConfig is undefined.`)
+    throw new Error(`${errorMsgPrefix} Could not get loader. nlsConfig is undefined.`)
   }
 
-  const requireOnSelf: RequireOnSelfType = {
+  const loader: Loader = {
     // Without the full URL VS Code will try to load file://.
     baseUrl: `${origin}${options.csStaticBase}/lib/vscode/out`,
     recordStats: true,
@@ -120,12 +111,7 @@ export function registerRequireOnSelf({ self, origin, nlsConfig, options }: Regi
     "vs/nls": nlsConfig,
   }
 
-  // TODO@jsjoeio
-  // I'm not sure how to properly type cast this
-  // This might be our best bet
-  // Source: https://stackoverflow.com/a/30740935
-  type FixMeLater = any
-  ;(self.require as FixMeLater) = requireOnSelf
+  return loader
 }
 
 try {
@@ -148,7 +134,12 @@ try {
         .catch(cb)
     }
   }
-  registerRequireOnSelf({
+  // TODO@jsjoeio
+  // I'm not sure how to properly type cast this
+  // This might be our best bet
+  // Source: https://stackoverflow.com/a/30740935
+  type FixMeLater = any
+  ;(self.require as FixMeLater) = getLoader({
     self,
     nlsConfig,
     options,

test/unit/browser/pages/vscode.test.ts

@@ -5,7 +5,7 @@ import { JSDOM } from "jsdom"
 import {
   getNlsConfiguration,
   nlsConfigElementId,
-  registerRequireOnSelf,
+  getLoader,
   setBodyBackgroundToThemeBackgroundColor,
 } from "../../../../src/browser/pages/vscode"
 
@@ -176,31 +176,7 @@ describe("vscode", () => {
       localStorage.removeItem("colorThemeData")
     })
   })
-  describe("registerRequireOnSelf", () => {
-    it("should throw an error if self is undefined", () => {
-      const options = {
-        base: "/",
-        csStaticBase: "/hello",
-        logLevel: 1,
-      }
-      const nlsConfig = {
-        first: "Jane",
-        last: "Doe",
-        locale: "en",
-        availableLanguages: {},
-      }
-      const errorMsgPrefix = "[vscode]"
-      const errorMessage = `${errorMsgPrefix} Could not register require on self. self is undefined.`
-      expect(() => {
-        registerRequireOnSelf({
-          // @ts-expect-error We are checking what happens if self is undefined.
-          self: undefined,
-          window: global.window,
-          nlsConfig: nlsConfig,
-          options,
-        })
-      }).toThrowError(errorMessage)
-    })
+  describe("getLoader", () => {
     it("should throw an error if window is undefined", () => {
       const options = {
         base: "/",
@@ -214,11 +190,9 @@ describe("vscode", () => {
         availableLanguages: {},
       }
       const errorMsgPrefix = "[vscode]"
-      const errorMessage = `${errorMsgPrefix} Could not register require on self. origin is undefined or missing.`
-      const mockSelf = {} as Window & typeof globalThis
+      const errorMessage = `${errorMsgPrefix} Could not get loader. origin is undefined or missing.`
       expect(() => {
-        registerRequireOnSelf({
-          self: mockSelf,
+        getLoader({
           // @ts-expect-error We need to test if window is undefined
           origin: undefined,
           nlsConfig: nlsConfig,
@@ -239,19 +213,16 @@ describe("vscode", () => {
         availableLanguages: {},
       }
       const errorMsgPrefix = "[vscode]"
-      const errorMessage = `${errorMsgPrefix} Could not register require on self. options or options.csStaticBase is undefined or missing.`
-      const mockSelf = {} as Window & typeof globalThis
+      const errorMessage = `${errorMsgPrefix} Could not get loader. options or options.csStaticBase is undefined or missing.`
       expect(() => {
-        registerRequireOnSelf({
-          self: mockSelf,
+        getLoader({
           origin: "localhost",
           nlsConfig: nlsConfig,
           options,
         })
       }).toThrowError(errorMessage)
       expect(() => {
-        registerRequireOnSelf({
-          self: mockSelf,
+        getLoader({
           origin: "localhost",
           nlsConfig: nlsConfig,
           // @ts-expect-error We need to check what happens when options is undefined
@@ -266,19 +237,17 @@ describe("vscode", () => {
         logLevel: 1,
       }
       const errorMsgPrefix = "[vscode]"
-      const errorMessage = `${errorMsgPrefix} Could not register require on self. nlsConfig is undefined.`
-      const mockSelf = {} as Window & typeof globalThis
+      const errorMessage = `${errorMsgPrefix} Could not get loader. nlsConfig is undefined.`
       expect(() => {
-        registerRequireOnSelf({
-          self: mockSelf,
+        getLoader({
           origin: "localthost",
           // @ts-expect-error We need to check that it works when this is undefined
           nlsConfig: undefined,
           options,
         })
       }).toThrowError(errorMessage)
     })
-    it("should declare require on self", () => {
+    it("should return a loader object", () => {
       const options = {
         base: "/",
         csStaticBase: "/",
@@ -290,16 +259,33 @@ describe("vscode", () => {
         locale: "en",
         availableLanguages: {},
       }
-      const mockSelf = {} as Window & typeof globalThis
-      registerRequireOnSelf({
-        self: mockSelf,
-        origin: "localthost",
+      const loader = getLoader({
+        origin: "localhost",
         nlsConfig: nlsConfig,
         options,
       })
 
-      const hasRequireProperty = Object.prototype.hasOwnProperty.call(mockSelf, "require")
-      expect(hasRequireProperty).toBeTruthy()
+      expect(loader).toStrictEqual({
+        baseUrl: "localhost//lib/vscode/out",
+        paths: {
+          "iconv-lite-umd": "../node_modules/iconv-lite-umd/lib/iconv-lite-umd.js",
+          jschardet: "../node_modules/jschardet/dist/jschardet.min.js",
+          "tas-client-umd": "../node_modules/tas-client-umd/lib/tas-client-umd.js",
+          "vscode-oniguruma": "../node_modules/vscode-oniguruma/release/main",
+          "vscode-textmate": "../node_modules/vscode-textmate/release/main",
+          xterm: "../node_modules/xterm/lib/xterm.js",
+          "xterm-addon-search": "../node_modules/xterm-addon-search/lib/xterm-addon-search.js",
+          "xterm-addon-unicode11": "../node_modules/xterm-addon-unicode11/lib/xterm-addon-unicode11.js",
+          "xterm-addon-webgl": "../node_modules/xterm-addon-webgl/lib/xterm-addon-webgl.js",
+        },
+        recordStats: true,
+        "vs/nls": {
+          availableLanguages: {},
+          first: "Jane",
+          last: "Doe",
+          locale: "en",
+        },
+      })
     })
   })
 })