Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Oct 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/fabric-website-resources/package.json
Expand Up @@ -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",
Expand Down
8 changes: 0 additions & 8 deletions apps/fabric-website-resources/src/index.bundle.ts

This file was deleted.

4 changes: 4 additions & 0 deletions 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);
49 changes: 25 additions & 24 deletions 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')
];
33 changes: 18 additions & 15 deletions 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: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addMonacoWebpackConfig requires entry to be an object

[BUNDLE_NAME]: './src/index.tsx'
},

externals: {
react: 'React',
'react-dom': 'ReactDOM'
},
externals: {
react: 'React',
'react-dom': 'ReactDOM'
},

resolve: {
alias: getResolveAlias()
}
});
resolve: {
alias: getResolveAlias()
}
})
);
3 changes: 2 additions & 1 deletion apps/fabric-website/package.json
Expand Up @@ -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",
Expand Down
@@ -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"
}
@@ -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"
}
@@ -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"
}
8 changes: 8 additions & 0 deletions 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"
}
2 changes: 2 additions & 0 deletions packages/monaco-editor/README.md
Expand Up @@ -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.
26 changes: 22 additions & 4 deletions 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:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a PR in monaco-typescript to make this slightly easier, but for now it seemed good to specify the full manual process.

// 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';
Expand All @@ -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;
Expand All @@ -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[];
Expand All @@ -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>;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>;
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/monaco-editor/package.json
Expand Up @@ -15,11 +15,11 @@
"just": "just-scripts"
},
"devDependencies": {
"monaco-editor": "0.18.1",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

monaco shouldn't be a production dependency since we're copying out all the relevant files and re-publishing

"@uifabric/build": "^7.0.0"
},
"dependencies": {
"@microsoft/load-themed-styles": "^1.7.13",
"monaco-editor": "0.17.1",
"tslib": "^1.7.1"
},
"optionalPeerDependencies": {
Expand Down
17 changes: 16 additions & 1 deletion packages/monaco-editor/scripts/addMonacoWebpackConfig.js
Expand Up @@ -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,
Expand All @@ -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
Expand Down
24 changes: 17 additions & 7 deletions packages/monaco-editor/src/configureEnvironment.ts
Expand Up @@ -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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
1 change: 1 addition & 0 deletions packages/tsx-editor/package.json
Expand Up @@ -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"
Expand Down
8 changes: 5 additions & 3 deletions packages/tsx-editor/src/components/TsxEditor.tsx
Expand Up @@ -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`);
})
);

Expand Down