[Monaco] Refactor the way XJSON grammar checker gets registered by jloleysens · Pull Request #75160 · elastic/kibana

jloleysens · 2020-08-17T13:44:29Z

Summary

This contribution makes the following changes:

avoid registering multiple model add listeners
remove regsiterGrammarChecker from public API
fix getWorker handler to register XJSON only for XJSON models

This fixes the workers ability to provide completions for multiple editor instances (two and more) that are using XJSON and it also ensure that we do not double-register a listener (or more times).

How to test

There are not really any instances of the monaco editor being used twice+ simultaneously. I would recommend making the following code changes to test:

Replace the contents of x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/custom.tsx with the code below. This will make the "custom" processor configuration form have 2 editors
Go to Ingest Node Pipelines plugin in the Stack Management section
Edit or create a pipeline
Add a new processor and type in "custom" in the type field and press enter
You should see the two text editors in the flyout
(Bonus) open your browser console, type option+cmd+f and enter XJSON_GRAMMAR_CHECKER in the search box. Select the instance in the language.js file. Place a breakpoint inside of the onDidChangeContent handler (this should be a single call to updateAnnos. Type something in the editor, this should be called once only, even after opening and closing the flyout.

Code

/*
 * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
 * or more contributor license agreements. Licensed under the Elastic License;
 * you may not use this file except in compliance with the Elastic License.
 */

import React, { FunctionComponent } from 'react';
import { i18n } from '@kbn/i18n';

import {
  FieldConfig,
  FIELD_TYPES,
  fieldValidators,
  UseField,
} from '../../../../../../shared_imports';

const { emptyField, isJsonField } = fieldValidators;

import { XJsonEditor } from '../field_components';

const customConfig: FieldConfig = {
  type: FIELD_TYPES.TEXT,
  label: i18n.translate('xpack.ingestPipelines.pipelineEditor.customForm.optionsFieldLabel', {
    defaultMessage: 'Configuration',
  }),
  serializer: (value: string) => {
    try {
      return JSON.parse(value);
    } catch (error) {
      // swallow error and return non-parsed value;
      return value;
    }
  },
  deserializer: (value: any) => {
    if (value === '') {
      return '{\n\n}';
    }
    return JSON.stringify(value, null, 2);
  },
  validations: [
    {
      validator: emptyField(
        i18n.translate(
          'xpack.ingestPipelines.pipelineEditor.customForm.configurationRequiredError',
          {
            defaultMessage: 'Configuration is required.',
          }
        )
      ),
    },
    {
      validator: isJsonField(
        i18n.translate('xpack.ingestPipelines.pipelineEditor.customForm.invalidJsonError', {
          defaultMessage: 'The input is not valid.',
        })
      ),
    },
  ],
};

interface Props {
  defaultOptions?: any;
}

/**
 * This is a catch-all component to support settings for custom processors
 * or existing processors not yet supported by the UI.
 *
 * We store the settings in a field called "customOptions"
 **/
export const Custom: FunctionComponent<Props> = ({ defaultOptions }) => {
  return (
    <>
      <UseField
        path="customOptions"
        component={XJsonEditor}
        config={customConfig}
        defaultValue={defaultOptions}
        componentProps={{
          editorProps: {
            'data-test-subj': 'processorOptionsEditor',
            height: 300,
            'aria-label': i18n.translate(
              'xpack.ingestPipelines.pipelineEditor.customForm.optionsFieldAriaLabel',
              {
                defaultMessage: 'Configuration JSON editor',
              }
            ),
          },
        }}
      />
      <UseField
        path="whatHaveYou"
        component={XJsonEditor}
        config={customConfig}
        defaultValue={defaultOptions}
        componentProps={{
          editorProps: {
            'data-test-subj': 'processorOptionsEditor',
            height: 300,
            'aria-label': i18n.translate(
              'xpack.ingestPipelines.pipelineEditor.customForm.optionsFieldAriaLabel',
              {
                defaultMessage: 'Configuration JSON editor',
              }
            ),
          },
        }}
      />
    </>
  );
};

- avoid registering multiple model add listeners - remove regsiterGrammarChecker from public API! - fix getWorker handler to register XJSON only for XJSON models

elasticmachine · 2020-08-17T13:44:32Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

sebelga

LGTM! Tested locally and made the 2 editor work.

Type something in the editor, this should be called once only

I have put the breakpoint but it didn't get called when changing the editor value. Even after refreshing the browser window. Maybe we can zoom about that?

sebelga · 2020-08-17T15:35:24Z

...ipeline_processors_editor/components/manage_processor_form/field_components/xjson_editor.tsx

 */
-
-import { XJsonLang } from '@kbn/monaco';
+import { XJsonLang, monaco } from '@kbn/monaco';


nit: you added the import but don't use it anywhere

kibanamachine · 2020-08-18T10:15:05Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c242a89

Build metrics

async chunks size

id	value	diff	baseline
ingestPipelines	643.8KB	-116.0B	643.9KB

History

💚 Build #68501 succeeded 74e5534
💔 Build #68437 failed 42a9fef

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

…tic#75160) * Refactor the way XJSON grammar checker gets registered - avoid registering multiple model add listeners - remove regsiterGrammarChecker from public API! - fix getWorker handler to register XJSON only for XJSON models * remove unused import * updateAnnos -> updateAnnotations

…tic#75160) * Refactor the way XJSON grammar checker gets registered - avoid registering multiple model add listeners - remove regsiterGrammarChecker from public API! - fix getWorker handler to register XJSON only for XJSON models * remove unused import * updateAnnos -> updateAnnotations # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/field_components/xjson_editor.tsx

…) (#75288) * Refactor the way XJSON grammar checker gets registered - avoid registering multiple model add listeners - remove regsiterGrammarChecker from public API! - fix getWorker handler to register XJSON only for XJSON models * remove unused import * updateAnnos -> updateAnnotations

* master: Skip failing test in CI (elastic#75266) [Task Manager] time out work when it overruns in poller (elastic#74980) [Drilldowns] misc improvements & fixes (elastic#75276) Small README note on bumping memory for builds (elastic#75247) [Security Solution][Detections] Adds exception modal tests (elastic#74596) [Dashboard] Sample data link does not work (elastic#75262) [Dashboard First] Unlink from Library Action With ReferenceOrValueEmbeddable (elastic#74905) [Form lib] Fix issue where serializer on fields are called on every change (elastic#75166) convert processor labels to sentence case (elastic#75278) [Monaco] Refactor the way XJSON grammar checker gets registered (elastic#75160) Clarify no documents error message when filtering by is_training (elastic#75227) [Lens] Fix crash when two layers xychart switches to pie (elastic#75267) [Observability Homepage] Fix console error because of side effect (elastic#75258) [Usage Collection] Add `legacy=true` option to the /api/stats request in the docs (elastic#75146) [ML] Functional tests - re-activate DFA test suites (elastic#75257) GS providers improvements (elastic#75174) [Visualize] First version of by-value visualize editor (elastic#72256)

…#75160) (#75291) * [Monaco] Refactor the way XJSON grammar checker gets registered (#75160) * Refactor the way XJSON grammar checker gets registered - avoid registering multiple model add listeners - remove regsiterGrammarChecker from public API! - fix getWorker handler to register XJSON only for XJSON models * remove unused import * updateAnnos -> updateAnnotations # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/field_components/xjson_editor.tsx * remove use of registerGrammarChecker in XJsonEditor

…emove-header * saved-objects/version-on-create: (59 commits) remove version when loading sample data omit version from SO import/export Skip failing test in CI (elastic#75266) [Task Manager] time out work when it overruns in poller (elastic#74980) [Drilldowns] misc improvements & fixes (elastic#75276) Small README note on bumping memory for builds (elastic#75247) [Security Solution][Detections] Adds exception modal tests (elastic#74596) Revert "Revert "added missing core docs"" Revert "Revert "added version to saved object bulk creation"" [Dashboard] Sample data link does not work (elastic#75262) [Dashboard First] Unlink from Library Action With ReferenceOrValueEmbeddable (elastic#74905) [Form lib] Fix issue where serializer on fields are called on every change (elastic#75166) convert processor labels to sentence case (elastic#75278) [Monaco] Refactor the way XJSON grammar checker gets registered (elastic#75160) Clarify no documents error message when filtering by is_training (elastic#75227) [Lens] Fix crash when two layers xychart switches to pie (elastic#75267) [Observability Homepage] Fix console error because of side effect (elastic#75258) [Usage Collection] Add `legacy=true` option to the /api/stats request in the docs (elastic#75146) [ML] Functional tests - re-activate DFA test suites (elastic#75257) GS providers improvements (elastic#75174) ...

Refactor the way XJSON grammar checker gets registered

42a9fef

- avoid registering multiple model add listeners - remove regsiterGrammarChecker from public API! - fix getWorker handler to register XJSON only for XJSON models

jloleysens added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes Feature:Ingest Node Pipelines Ingest node pipelines management v7.9.1 labels Aug 17, 2020

jloleysens requested a review from sebelga August 17, 2020 13:44

jloleysens requested a review from a team as a code owner August 17, 2020 13:44

sebelga approved these changes Aug 17, 2020

View reviewed changes

jloleysens added 2 commits August 17, 2020 17:53

remove unused import

74e5534

updateAnnos -> updateAnnotations

c242a89

jloleysens merged commit 36f0c75 into elastic:master Aug 18, 2020

jloleysens deleted the kbn-monaco/fix/handle-multiple-editor-instances branch August 18, 2020 13:13

jloleysens added the v7.10.0 label Aug 18, 2020

jloleysens mentioned this pull request Aug 18, 2020

[7.x] [Monaco] Refactor the way XJSON grammar checker gets registered (#75160) #75288

Merged

jloleysens mentioned this pull request Aug 18, 2020

[7.9] [Monaco] Refactor the way XJSON grammar checker gets registered (#75160) #75291

Merged

jloleysens mentioned this pull request Aug 19, 2020

[Ingest Pipelines] Processor forms for processors E-J #75054

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Monaco] Refactor the way XJSON grammar checker gets registered#75160

[Monaco] Refactor the way XJSON grammar checker gets registered#75160
jloleysens merged 3 commits intoelastic:masterfrom
jloleysens:kbn-monaco/fix/handle-multiple-editor-instances

jloleysens commented Aug 17, 2020

Uh oh!

elasticmachine commented Aug 17, 2020

Uh oh!

sebelga left a comment

Uh oh!

sebelga Aug 17, 2020

Uh oh!

kibanamachine commented Aug 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jloleysens commented Aug 17, 2020

Summary

How to test

Uh oh!

elasticmachine commented Aug 17, 2020

Uh oh!

sebelga left a comment

Choose a reason for hiding this comment

Uh oh!

sebelga Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Aug 18, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants