-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Conversation
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: d78a5c1de55316f488dd77386ba6cb33ac2da779 (build) |
Component Perf AnalysisNo significant results to display. All results
|
@@ -1,8 +0,0 @@ | |||
export * from './index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this wasn't being used for anything
}, | ||
module.exports = resources.createServeConfig( | ||
addMonacoWebpackConfig({ | ||
entry: { |
There was a problem hiding this comment.
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
@@ -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: |
There was a problem hiding this comment.
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.
@@ -15,11 +15,11 @@ | |||
"just": "just-scripts" | |||
}, | |||
"devDependencies": { | |||
"monaco-editor": "0.18.1", |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would really prefer not having to add the raw-loader dep but it was already the case until we can do a better implementation with library build time raw-loader via something with node
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
Pull request checklist
$ yarn change
Description of changes
To fix loading the editor's workers in Edge (part of Live editor shows errors for some examples/in some browsers #10937), try loading blob URLs instead of manually-constructed data URLs, as suggested here.
To fix the invalid hook errors that only showed up in the website (other part of Live editor shows errors for some examples/in some browsers #10937), update the code generation to return a function which is called with the correct version of React. The hook error was because the eval'd example was using the website's global version of React instead of the bundled version.
Enable the editor in the OUFR demo app. (We can't just enable it for all demo apps automatically due to the extra requirements for setting up typings.)
Update Monaco to fix Documentation: Code Examples in Control-Documentation no longer scrollable using mouse wheel in Firefox #10865. This required a workaround in the webpack config for a newly-introduced bundling problem (see monaco issue for details).
Fix a couple other minor editor bugs, including handling either string or default exports from raw-loader imports
Microsoft Reviewers: Open in CodeFlow