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

[Monaco] Add JSON syntax support to the Monaco editor #143739

Merged
merged 5 commits into from
Nov 1, 2022

Conversation

dokmic
Copy link
Contributor

@dokmic dokmic commented Oct 20, 2022

Summary

Resolves #107733 and #138599.

The PR adds JSON syntax support to the monaco editor.
A custom schema can be defined per editor instance using the editorDidMount life-cycle hook in the CodeEditor component.

<CodeEditor
  languageId="json"
  editorDidMount={(editor: monaco.editor.IStandaloneCodeEditor) => {
    monaco.languages.json.jsonDefaults.setDiagnosticsOptions({
      validate: true,
      schemas: [
        {
          uri: editor.getModel()?.uri.toString() ?? '',
          fileMatch: ['*'],
          schema: {
            type: 'object',
            properties: {
              version: {
                enum: ['v1', 'v2'],
              },
            },
          },
        },
      ],
    });
  }}
/>

@dokmic dokmic added review release_note:skip Skip the PR/issue when compiling release notes Team:AppServicesUx v8.6.0 labels Oct 20, 2022
@dokmic dokmic marked this pull request as ready for review October 20, 2022 11:41
@dokmic dokmic requested a review from a team as a code owner October 20, 2022 11:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

This is awesome! I'm approving, but it would be nice to test this out somewhere. Perhaps as a separate issue we can create a storybook of the monaco editor with our different languages.

@dokmic
Copy link
Contributor Author

dokmic commented Oct 24, 2022

Bumped the monaco-editor package to 0.24.0 to resolve microsoft/monaco-editor#2460.

@dokmic dokmic requested a review from a team October 25, 2022 19:40
@dokmic dokmic requested a review from a team as a code owner October 25, 2022 19:40
Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

These changes LGTM - can you add a story example for the kibana_react storybook? I'm thinking something like

add(
    'json support',
    () => (
      <div>
      <CodeEditor
        languageId="json"
        editorDidMount={(editor: monaco.editor.IStandaloneCodeEditor) => {
          monaco.languages.json.jsonDefaults.setDiagnosticsOptions({
            validate: true,
            schemas: [
              {
                uri: editor.getModel()?.uri.toString() ?? '',
                fileMatch: ['*'],
                schema: {
                  type: 'object',
                  properties: {
                    version: {
                      enum: ['v1', 'v2'],
                    },
                  },
                },
              },
            ],
          });
        }}
        height={250}
        value="{}"
        onChange={action('onChange')}
      />
    </div>
  ),
  {
    info: {
      text: 'JSON language support',
    },
  })

?
Thanks

@dokmic dokmic requested a review from a team as a code owner October 27, 2022 08:49
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

@elastic/kibana-vis-editors checked Lens formula and works fine 👍

@dokmic dokmic force-pushed the feature/107733 branch 2 times, most recently from ee6e54d to 03c8754 Compare October 28, 2022 14:49
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

code editor changes LGTM! Code only review

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #12 / Actions and Triggers app create alert "after each" hook for "should create an alert"
  • [job] [logs] FTR Configs #12 / Actions and Triggers app create alert should create an alert

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
kibanaReact 208.1KB 208.1KB +23.0B
lens 1.3MB 1.3MB +61.0B
presentationUtil 130.5KB 130.6KB +86.0B
total +170.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-css 437.3KB 438.5KB +1.1KB
kbnUiSharedDeps-fonts 61.3KB 69.3KB +8.0KB
kbnUiSharedDeps-srcJs 3.8MB 4.1MB ⚠️ +299.2KB
total +308.3KB
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
@kbn/monaco 2 3 +1
enterpriseSearch 19 21 +2
fleet 57 63 +6
osquery 103 108 +5
securitySolution 439 443 +4
total +18

Total ESLint disabled count

id before after diff
@kbn/monaco 6 7 +1
enterpriseSearch 20 22 +2
fleet 65 71 +6
osquery 104 110 +6
securitySolution 516 520 +4
total +19

History

  • 💚 Build #83958 succeeded e25b19700fc3d795a67eea45ca11a428a2e9b1d5
  • 💚 Build #83570 succeeded 03c8754dcf15d682ba3c0a8bb15fc8148973fc06
  • 💔 Build #83464 failed ee6e54d4e6c1491a5afca5dec9557a6247bd6574
  • 💚 Build #83078 succeeded 7643c5abe6a043e9ddd82e07269386fbef773324
  • 💔 Build #82805 failed 8679976705e3b86a2526dd5be0d42e6723c91be3
  • 💔 Build #82624 failed 589734f5e7ee878f291c92f532387fe16d94eaca

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dokmic dokmic merged commit 9456303 into elastic:main Nov 1, 2022
@dokmic dokmic deleted the feature/107733 branch November 1, 2022 07:54
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 1, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 1, 2022
* main: (43 commits)
  [Synthetics] Step details page screenshot (elastic#143452)
  [Lens] Datatable expression types improvement. (elastic#144173)
  [packages/kbn-journeys] start apm after browser start and stop after browser is closed (elastic#144267)
  [Files] Make files namespace agnostic (elastic#144019)
  Implement base browser-side logging system (elastic#144107)
  Correct wrong multiplier for byte conversion (elastic#143751)
  [Monaco] Add JSON syntax support to the Monaco editor (elastic#143739)
  CCS Smoke Test for Remote Clusters and Index Management  (elastic#142423)
  [api-docs] Daily api_docs build (elastic#144294)
  chore(NA): include progress on Bazel tasks (elastic#144275)
  [RAM] Allow users to see event logs from all spaces they have access to (elastic#140449)
  [APM] Show recommended minimum size when going below 5 minutes (elastic#144170)
  [typecheck] delete temporary target_types dirs in packages (elastic#144271)
  [Security Solution][Endpoint] adds new alert loading utility and un-skip FTR test for endpoint (elastic#144133)
  [performance/journeys] revert data_stress_test_lens.ts journey step (elastic#144261)
  [TIP] Use search strategies in Threat Intelligence (elastic#143267)
  Optimize react-query dependencies (elastic#144206)
  [babel/node] invalidate cache when synth pkg map is updated (elastic#144258)
  [APM] AWS lambda estimated cost (elastic#143986)
  [Maps] layer group wizard (elastic#144129)
  ...
nickpeihl added a commit that referenced this pull request Nov 28, 2022
Fixes #146243 

## Summary

Fixes Canvas expression autocomplete

#143739 upgraded the monaco-editor
dependency which uses a callback to the `onLanguage` method to
initialize the expressions. The PR moved the `monaco.languages.register`
command inside this callback and which was never triggered.

Moving the `monaco.languages.register` command outside the callback
appears to fix the issue.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 28, 2022
Fixes elastic#146243

## Summary

Fixes Canvas expression autocomplete

elastic#143739 upgraded the monaco-editor
dependency which uses a callback to the `onLanguage` method to
initialize the expressions. The PR moved the `monaco.languages.register`
command inside this callback and which was never triggered.

Moving the `monaco.languages.register` command outside the callback
appears to fix the issue.

(cherry picked from commit 19413b7)
kibanamachine added a commit that referenced this pull request Nov 28, 2022
…#146465)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[PresentationUtil] Fix Canvas expression autocomplete
(#146425)](#146425)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nick
Peihl","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-11-28T19:30:37Z","message":"[PresentationUtil]
Fix Canvas expression autocomplete (#146425)\n\nFixes #146243 \r\n\r\n##
Summary\r\n\r\nFixes Canvas expression
autocomplete\r\n\r\nhttps://github.com//pull/143739
upgraded the monaco-editor\r\ndependency which uses a callback to the
`onLanguage` method to\r\ninitialize the expressions. The PR moved the
`monaco.languages.register`\r\ncommand inside this callback and which
was never triggered.\r\n\r\nMoving the `monaco.languages.register`
command outside the callback\r\nappears to fix the
issue.","sha":"19413b7daae983b95dbb9f5c7b39cb8f3578ebfa","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","backport:prev-minor","v8.6.0","v8.7.0"],"number":146425,"url":"https://github.com/elastic/kibana/pull/146425","mergeCommit":{"message":"[PresentationUtil]
Fix Canvas expression autocomplete (#146425)\n\nFixes #146243 \r\n\r\n##
Summary\r\n\r\nFixes Canvas expression
autocomplete\r\n\r\nhttps://github.com//pull/143739
upgraded the monaco-editor\r\ndependency which uses a callback to the
`onLanguage` method to\r\ninitialize the expressions. The PR moved the
`monaco.languages.register`\r\ncommand inside this callback and which
was never triggered.\r\n\r\nMoving the `monaco.languages.register`
command outside the callback\r\nappears to fix the
issue.","sha":"19413b7daae983b95dbb9f5c7b39cb8f3578ebfa"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/146425","number":146425,"mergeCommit":{"message":"[PresentationUtil]
Fix Canvas expression autocomplete (#146425)\n\nFixes #146243 \r\n\r\n##
Summary\r\n\r\nFixes Canvas expression
autocomplete\r\n\r\nhttps://github.com//pull/143739
upgraded the monaco-editor\r\ndependency which uses a callback to the
`onLanguage` method to\r\ninitialize the expressions. The PR moved the
`monaco.languages.register`\r\ncommand inside this callback and which
was never triggered.\r\n\r\nMoving the `monaco.languages.register`
command outside the callback\r\nappears to fix the
issue.","sha":"19413b7daae983b95dbb9f5c7b39cb8f3578ebfa"}}]}]
BACKPORT-->

Co-authored-by: Nick Peihl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes review v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support JSON in kibana_react CodeEditor
8 participants