Editor: fix bugs, enable in demo app, update Monaco (#10973)

* Update Monaco and enable editor in OUFR demo app * Change files * specify raw-loader dep * Fix invalid hook error
microsoft · Oct 28, 2019 · d2ddc7b · d2ddc7b
1 parent d78a5c1
commit d2ddc7b
Show file tree

Hide file tree

Showing 24 changed files with 191 additions and 94 deletions.
diff --git a/apps/fabric-website-resources/package.json b/apps/fabric-website-resources/package.json
@@ -71,6 +71,7 @@
     "@uifabric/set-version": "^7.0.2",
     "@uifabric/styling": "^7.7.2",
     "@uifabric/theme-samples": "^7.0.5",
+    "@uifabric/tsx-editor": "^0.10.2",
     "@uifabric/utilities": "^7.5.0",
     "@uifabric/variants": "^7.0.5",
     "expect-puppeteer": "4.1.0",

diff --git a/apps/fabric-website-resources/src/index.bundle.ts b/apps/fabric-website-resources/src/index.bundle.ts
diff --git a/apps/fabric-website-resources/src/index.tsx b/apps/fabric-website-resources/src/index.tsx
@@ -1,5 +1,9 @@
 import { createDemoApp } from '@uifabric/example-app-base';
+import { configureEnvironment } from '@uifabric/tsx-editor';
 import { AppDefinition } from './AppDefinition';
 import { GettingStartedPage } from './GettingStartedPage';
 
+// Configure example editor
+configureEnvironment({ baseUrl: '.', useMinified: false });
+
 createDemoApp(AppDefinition, GettingStartedPage);
diff --git a/apps/fabric-website-resources/webpack.config.js b/apps/fabric-website-resources/webpack.config.js
@@ -1,37 +1,38 @@
 let path = require('path');
-const resources = require('../../scripts/webpack/webpack-resources');
+const resources = require('@uifabric/build/webpack/webpack-resources');
+const { addMonacoWebpackConfig } = require('@uifabric/tsx-editor/scripts/addMonacoWebpackConfig');
 
 const BUNDLE_NAME = 'office-ui-fabric-react';
 const IS_PRODUCTION = process.argv.indexOf('--production') > -1;
 
 module.exports = [
-  ...resources.createConfig(BUNDLE_NAME, IS_PRODUCTION, {
-    entry: { [BUNDLE_NAME]: './lib/index.bundle.js' },
+  ...resources.createConfig(
+    BUNDLE_NAME,
+    IS_PRODUCTION,
+    addMonacoWebpackConfig({
+      entry: { [BUNDLE_NAME]: './lib/index.js' },
 
-    output: {
-      libraryTarget: 'var',
-      library: 'Fabric'
-    },
-
-    externals: [
-      {
-        react: 'React'
+      output: {
+        libraryTarget: 'var',
+        library: 'Fabric'
       },
-      {
+
+      externals: {
+        react: 'React',
         'react-dom': 'ReactDOM'
-      }
-    ],
+      },
 
-    resolve: {
-      alias: {
-        'office-ui-fabric-react$': path.resolve(__dirname, '../../packages/office-ui-fabric-react/lib'),
-        'office-ui-fabric-react/src': path.resolve(__dirname, '../../packages/office-ui-fabric-react/src'),
-        'office-ui-fabric-react/lib': path.resolve(__dirname, '../../packages/office-ui-fabric-react/lib'),
-        '@uifabric/api-docs/lib': path.resolve(__dirname, '../../packages/api-docs/lib'),
-        'Props.ts.js': 'Props',
-        'Example.tsx.js': 'Example'
+      resolve: {
+        alias: {
+          'office-ui-fabric-react$': path.resolve(__dirname, '../../packages/office-ui-fabric-react/lib'),
+          'office-ui-fabric-react/src': path.resolve(__dirname, '../../packages/office-ui-fabric-react/src'),
+          'office-ui-fabric-react/lib': path.resolve(__dirname, '../../packages/office-ui-fabric-react/lib'),
+          '@uifabric/api-docs/lib': path.resolve(__dirname, '../../packages/api-docs/lib'),
+          'Props.ts.js': 'Props',
+          'Example.tsx.js': 'Example'
+        }
       }
-    }
-  }),
+    })
+  ),
   require('./webpack.serve.config')
 ];
diff --git a/apps/fabric-website-resources/webpack.serve.config.js b/apps/fabric-website-resources/webpack.serve.config.js
@@ -1,19 +1,22 @@
-const getResolveAlias = require('../../scripts/webpack/getResolveAlias');
-const resources = require('../../scripts/webpack/webpack-resources');
+const getResolveAlias = require('@uifabric/build/webpack/getResolveAlias');
+const resources = require('@uifabric/build/webpack/webpack-resources');
+const { addMonacoWebpackConfig } = require('@uifabric/tsx-editor/scripts/addMonacoWebpackConfig');
 
-module.exports = resources.createServeConfig({
-  entry: './src/index.tsx',
+const BUNDLE_NAME = 'demo-app';
 
-  output: {
-    filename: 'demo-app.js'
-  },
+module.exports = resources.createServeConfig(
+  addMonacoWebpackConfig({
+    entry: {
+      [BUNDLE_NAME]: './src/index.tsx'
+    },
 
-  externals: {
-    react: 'React',
-    'react-dom': 'ReactDOM'
-  },
+    externals: {
+      react: 'React',
+      'react-dom': 'ReactDOM'
+    },
 
-  resolve: {
-    alias: getResolveAlias()
-  }
-});
+    resolve: {
+      alias: getResolveAlias()
+    }
+  })
+);
diff --git a/apps/fabric-website/package.json b/apps/fabric-website/package.json
@@ -41,7 +41,8 @@
     "react-dom": "16.8.6",
     "react-highlight": "0.10.0",
     "write-file-webpack-plugin": "^4.1.0",
-    "@uifabric/build": "^7.0.0"
+    "@uifabric/build": "^7.0.0",
+    "@uifabric/tsx-editor": "^0.10.2"
   },
   "dependencies": {
     "@microsoft/load-themed-styles": "^1.7.13",

diff --git a/change/@uifabric-fabric-website-2019-10-26-12-51-04-tsx-fix.json b/change/@uifabric-fabric-website-2019-10-26-12-51-04-tsx-fix.json
@@ -0,0 +1,8 @@
+{
+  "type": "none",
+  "comment": "Add missing tsx-editor dev dep",
+  "packageName": "@uifabric/fabric-website",
+  "email": "elcraig@microsoft.com",
+  "commit": "da81d25a70517a242b31f2b25ee8b07975b6e539",
+  "date": "2019-10-26T19:41:23.322Z"
+}
diff --git a/change/@uifabric-fabric-website-resources-2019-10-26-12-51-04-tsx-fix.json b/change/@uifabric-fabric-website-resources-2019-10-26-12-51-04-tsx-fix.json
@@ -0,0 +1,8 @@
+{
+  "type": "minor",
+  "comment": "Enable live editor and update bundling",
+  "packageName": "@uifabric/fabric-website-resources",
+  "email": "elcraig@microsoft.com",
+  "commit": "da81d25a70517a242b31f2b25ee8b07975b6e539",
+  "date": "2019-10-26T19:41:10.088Z"
+}
diff --git a/change/@uifabric-monaco-editor-2019-10-26-12-51-04-tsx-fix.json b/change/@uifabric-monaco-editor-2019-10-26-12-51-04-tsx-fix.json
@@ -0,0 +1,8 @@
+{
+  "type": "minor",
+  "comment": "Pick up new Monaco version and try a different cross-domain worker loading method",
+  "packageName": "@uifabric/monaco-editor",
+  "email": "elcraig@microsoft.com",
+  "commit": "da81d25a70517a242b31f2b25ee8b07975b6e539",
+  "date": "2019-10-26T19:41:34.548Z"
+}
diff --git a/change/@uifabric-tsx-editor-2019-10-26-12-51-04-tsx-fix.json b/change/@uifabric-tsx-editor-2019-10-26-12-51-04-tsx-fix.json
@@ -0,0 +1,8 @@
+{
+  "type": "patch",
+  "comment": "Fix some minor bugs",
+  "packageName": "@uifabric/tsx-editor",
+  "email": "elcraig@microsoft.com",
+  "commit": "da81d25a70517a242b31f2b25ee8b07975b6e539",
+  "date": "2019-10-26T19:51:04.006Z"
+}
diff --git a/packages/monaco-editor/README.md b/packages/monaco-editor/README.md
@@ -79,3 +79,5 @@ If you need types from `monaco-typescript`, import as follows:
 ```js
 import { TypeScriptWorker } from '@uifabric/monaco-editor/monaco-typescript.d';
 ```
+
+Note that you may run into conflicts with these types if you're on a different TypeScript version than the one the typings were generated against.
diff --git a/packages/monaco-editor/monaco-typescript.d.ts b/packages/monaco-editor/monaco-typescript.d.ts
@@ -1,5 +1,10 @@
-// This file was generated by checking out and building monaco-typescript (https://github.com/Microsoft/monaco-typescript)
-// and merging release/esm/*.d.ts together, with minor edits.
+// This file may need to be re-generated when updating the monaco-editor version. Steps:
+// 1. Clone https://github.com/Microsoft/monaco-typescript
+// 2. npm install
+// 3. Edit src/tsconfig.json and src/tsconfig.esm.json to include "declaration": true
+// 4. npm run compile
+// 5. Merge .d.ts files from release/esm into this file (unfortunately a manual process right now)
+// 6. Resolve any type mismatch issues (likely caused by mismatches between our TS version and Monaco's)
 
 // merged imports from all files
 import * as ts from 'typescript';
@@ -19,14 +24,15 @@ import { Omit } from '@uifabric/utilities';
 export type EmitOutput = ts.EmitOutput;
 
 // languageFeatures.d.ts
+export declare function flattenDiagnosticMessageText(diag: string | ts.DiagnosticMessageChain | undefined, newLine: string, indent?: number): string;
 export declare abstract class Adapter {
   protected _worker: (first: Uri, ...more: Uri[]) => Promise<TypeScriptWorker>;
   constructor(_worker: (first: Uri, ...more: Uri[]) => Promise<TypeScriptWorker>);
   protected _positionToOffset(uri: Uri, position: monaco.IPosition): number;
   protected _offsetToPosition(uri: Uri, offset: number): monaco.IPosition;
   protected _textSpanToRange(uri: Uri, span: ts.TextSpan): monaco.IRange;
 }
-export declare class DiagnostcsAdapter extends Adapter {
+export declare class DiagnosticsAdapter extends Adapter {
   private _defaults;
   private _selector;
   private _disposables;
@@ -35,6 +41,7 @@ export declare class DiagnostcsAdapter extends Adapter {
   dispose(): void;
   private _doValidate;
   private _convertDiagnostics;
+  private _tsDiagnosticCategoryToMarkerSeverity;
 }
 export declare class SuggestAdapter extends Adapter implements monaco.languages.CompletionItemProvider {
   readonly triggerCharacters: string[];
@@ -44,7 +51,7 @@ export declare class SuggestAdapter extends Adapter implements monaco.languages.
 }
 export declare class SignatureHelpAdapter extends Adapter implements monaco.languages.SignatureHelpProvider {
   signatureHelpTriggerCharacters: string[];
-  provideSignatureHelp(model: monaco.editor.IReadOnlyModel, position: Position, token: CancellationToken): Thenable<monaco.languages.SignatureHelp>;
+  provideSignatureHelp(model: monaco.editor.IReadOnlyModel, position: Position, token: CancellationToken): Thenable<monaco.languages.SignatureHelpResult>;
 }
 export declare class QuickInfoAdapter extends Adapter implements monaco.languages.HoverProvider {
   provideHover(model: monaco.editor.IReadOnlyModel, position: Position, token: CancellationToken): Thenable<monaco.languages.Hover>;
@@ -105,6 +112,13 @@ export declare class FormatOnTypeAdapter extends FormatHelper implements monaco.
   readonly autoFormatTriggerCharacters: string[];
   provideOnTypeFormattingEdits(model: monaco.editor.IReadOnlyModel, position: Position, ch: string, options: monaco.languages.FormattingOptions, token: CancellationToken): Thenable<ISingleEditOperation[]>;
 }
+export declare class CodeActionAdaptor extends FormatHelper implements monaco.languages.CodeActionProvider {
+  provideCodeActions(model: monaco.editor.ITextModel, range: Range, context: monaco.languages.CodeActionContext, token: CancellationToken): Promise<monaco.languages.CodeActionList>;
+  private _tsCodeFixActionToMonacoCodeAction;
+}
+export declare class RenameAdapter extends Adapter implements monaco.languages.RenameProvider {
+  provideRenameEdits(model: monaco.editor.ITextModel, position: Position, newName: string, token: CancellationToken): Promise<monaco.languages.WorkspaceEdit & monaco.languages.Rejection>;
+}
 
 // monaco.contribution.d.ts
 export interface IExtraLib {
@@ -164,6 +178,7 @@ export declare class TypeScriptWorker implements ts.LanguageServiceHost {
   private static clearFiles;
   getSyntacticDiagnostics(fileName: string): Promise<ts.Diagnostic[]>;
   getSemanticDiagnostics(fileName: string): Promise<ts.Diagnostic[]>;
+  getSuggestionDiagnostics(fileName: string): Promise<ts.DiagnosticWithLocation[]>;
   getCompilerOptionsDiagnostics(fileName: string): Promise<ts.Diagnostic[]>;
   getCompletionsAtPosition(fileName: string, position: number): Promise<ts.CompletionInfo>;
   getCompletionEntryDetails(fileName: string, position: number, entry: string): Promise<ts.CompletionEntryDetails>;
@@ -176,7 +191,10 @@ export declare class TypeScriptWorker implements ts.LanguageServiceHost {
   getFormattingEditsForDocument(fileName: string, options: ts.FormatCodeOptions): Promise<ts.TextChange[]>;
   getFormattingEditsForRange(fileName: string, start: number, end: number, options: ts.FormatCodeOptions): Promise<ts.TextChange[]>;
   getFormattingEditsAfterKeystroke(fileName: string, postion: number, ch: string, options: ts.FormatCodeOptions): Promise<ts.TextChange[]>;
+  findRenameLocations(fileName: string, positon: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename: boolean): Promise<readonly ts.RenameLocation[]>;
+  getRenameInfo(fileName: string, positon: number, options: ts.RenameInfoOptions): Promise<ts.RenameInfo>;
   getEmitOutput(fileName: string): Promise<ts.EmitOutput>;
+  getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: ts.FormatCodeOptions): Promise<ReadonlyArray<ts.CodeFixAction>>;
   updateExtraLibs(extraLibs: IExtraLibs): void;
 }
 export interface ICreateData {

diff --git a/packages/monaco-editor/package.json b/packages/monaco-editor/package.json
@@ -15,11 +15,11 @@
     "just": "just-scripts"
   },
   "devDependencies": {
+    "monaco-editor": "0.18.1",
     "@uifabric/build": "^7.0.0"
   },
   "dependencies": {
     "@microsoft/load-themed-styles": "^1.7.13",
-    "monaco-editor": "0.17.1",
     "tslib": "^1.7.1"
   },
   "optionalPeerDependencies": {

diff --git a/packages/monaco-editor/scripts/addMonacoWebpackConfig.js b/packages/monaco-editor/scripts/addMonacoWebpackConfig.js
@@ -20,11 +20,25 @@ function addMonacoWebpackConfig(config, includeAllLanguages) {
     throw new Error('config passed to addMonacoConfig must be an object, not an array or function.');
   }
 
-  const { entry, output, resolve } = config;
+  const { entry, output, externals, resolve } = config;
   if (!entry || typeof entry !== 'object') {
     throw new Error(`config.entry passed to addMonacoWebpackConfig must be an object. Got: ${JSON.stringify(entry)}`);
   }
 
+  // As of monaco-editor@0.18.1, typescriptServices.js includes a direct require for this package,
+  // which breaks webpack. Use an external to get rid of it (this works since the require is
+  // wrapped in a try/catch). Will be fixed once this merges and is published.
+  // https://github.com/microsoft/monaco-typescript/pull/49
+  /** @type {webpack.ExternalsElement[]} */
+  const newExternals = [{ '@microsoft/typescript-etw': 'FakeModule' }];
+  if (externals) {
+    if (Array.isArray(externals)) {
+      newExternals.push(...externals);
+    } else {
+      newExternals.push(externals);
+    }
+  }
+
   // Somewhat based on https://github.com/microsoft/monaco-editor/blob/master/docs/integrate-esm.md
   return {
     ...config,
@@ -40,6 +54,7 @@ function addMonacoWebpackConfig(config, includeAllLanguages) {
           }
         : {})
     },
+    externals: newExternals,
     output: {
       ...output,
       globalObject: 'self' // required for monaco--see https://github.com/webpack/webpack/issues/6642

diff --git a/packages/monaco-editor/src/configureEnvironment.ts b/packages/monaco-editor/src/configureEnvironment.ts
@@ -4,8 +4,8 @@ export interface IMonacoConfig {
   /** Whether to use minified versions of the files (`.min.js`) */
   useMinified?: boolean;
   /**
-   * Whether to use a configuration variant which works when this script lives
-   * on a different domain than the core Monaco scripts
+   * Whether to use a configuration variant which works when the worker script lives on a
+   * different domain than the website host (like a CDN).
    */
   crossDomain?: boolean;
 }
@@ -28,7 +28,7 @@ const labelMap: { [key: string]: string } = {
   json: 'json'
 };
 
-export function getMonacoConfig(): IMonacoConfig | undefined {
+function getMonacoConfig(): IMonacoConfig | undefined {
   return (
     globalObj.MonacoConfig ||
     // TODO: remove once fabric-website homepage.htm is updated
@@ -69,13 +69,23 @@ export function configureEnvironment(config?: IMonacoConfig): void {
 
       if (crossDomain) {
         // This is needed for cases where the JS files will be on a different domain (the CDN)
-        // instead of the domain the HTML is running on. Web workers (used by Monaco) can't be
-        // loaded by script residing on a different domain, so we use this proxy script on the
-        // main domain to load the worker script. (Also do this with localhost/devhost for testing.)
+        // instead of the domain the HTML is running on. Web worker scripts can't be directly
+        // loaded from a different domain, so we use this proxy. More info:
         // https://github.com/microsoft/monaco-editor/blob/master/docs/integrate-amd-cross.md
-        return 'data:text/javascript;charset=utf-8,' + encodeURIComponent(`importScripts("${path}");`);
+        // Plot twist! The approach of manually building a data URI suggested by those docs
+        // didn't work in Edge, but this blob approach seems to work everywhere.
+        // https://benohead.com/cross-domain-cross-browser-web-workers/
+        const blob = new Blob([`importScripts("${path}")`], { type: 'application/javascript' });
+        return URL.createObjectURL(blob);
       }
       return path;
     }
   };
 }
+
+/**
+ * Returns true if either MonacoEnvironment or MonacoConfig is set.
+ */
+export function isConfigAvailable(): boolean {
+  return !!(globalObj.MonacoConfig || globalObj.MonacoEnvironment);
+}
diff --git a/packages/tsx-editor/package.json b/packages/tsx-editor/package.json
@@ -48,6 +48,7 @@
     "@uifabric/monaco-editor": "^0.3.0",
     "@uifabric/react-hooks": "^7.0.1",
     "office-ui-fabric-react": "^7.55.2",
+    "raw-loader": "^0.5.1",
     "react-syntax-highlighter": "^10.1.3",
     "tslib": "^1.7.1",
     "typescript": "3.5.1"

diff --git a/packages/tsx-editor/src/components/TsxEditor.tsx b/packages/tsx-editor/src/components/TsxEditor.tsx
@@ -141,9 +141,11 @@ function _loadTypes(supportedPackages: IPackageGroup[]): Promise<void> {
   // React must be loaded first
   promises.push(
     // @ts-ignore: this import is handled by webpack
-    import('!raw-loader!@types/react/index.d.ts') // prettier-ignore
-      .then((result: { default: string }) => {
-        typescriptDefaults.addExtraLib(result.default, `${typesPrefix}/react/index.d.ts`);
+    import('!raw-loader!@types/react/index.d.ts')
+      // raw-loader 0.x exports a single string, and later versions export a default.
+      // The package.json specifies 0.x, but handle either just in case.
+      .then((result: string | { default: string }) => {
+        typescriptDefaults.addExtraLib(typeof result === 'string' ? result : result.default, `${typesPrefix}/react/index.d.ts`);
       })
   );