Skip to content

Commit

Permalink
[8.17] [Vega] Fix highlight for HJSON (#208858) (#209289)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.17`:
- [[Vega] Fix highlight for HJSON
(#208858)](#208858)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2025-02-03T13:18:35Z","message":"[Vega]
Fix highlight for HJSON (#208858)\n\n## Summary\r\n\r\nThis PR fixes the
missing highlight theme for HJSON specs in Vega.\r\nThe issue
#205711 was caused
by\r\nhttps://github.com//pull/182348 that
inadvertently\r\nspecified the code-editor package as sideEffect free.
This cause the\r\ncompiler to exclude every import without exported and
used methods.\r\nThis was the case for the code-editor that registered
some language\r\nhighlighters in that way.\r\n\r\nThe solution adopted
here is to mark the register_language.ts file as a\r\nfile with side
effects, the alternative solution can be to register\r\nthese directly
from within the code_editor component.\r\nA third option is to move
these registration within the monaco package\r\nwhere other languages
are also registered.\r\nI'd like to leave to @elastic/appex-sharedux the
preference to followup\r\nwith a better fix for the future.\r\n\r\nI'd
also like to have the opinion from @elastic/kibana-operations\r\nbecause
the misconfigured package issue can be seen only in production\r\nand
not in development mode. Is it possible that webpack doesn't
apply\r\ntreeshaking when in development mode?\r\n\r\nfix
https://github.com/elastic/kibana/issues/205711\r\nShould also fix the
same issue but for TSVB Markdown\r\n\r\nThe fix was tested on CI by
running at first only the CI FT with the\r\n`sideEffects:false` to
verify the failure
(see\r\n[build](https://buildkite.com/elastic/kibana-pull-request/builds/272375))\r\nand
then with the fix specifying the actual file that contains
side\r\neffects.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:TSVB","release_note:fix","Feature:Vega","Team:Visualizations","backport:prev-major","v9.1.0"],"title":"[Vega]
Fix highlight for
HJSON","number":208858,"url":"https://github.com/elastic/kibana/pull/208858","mergeCommit":{"message":"[Vega]
Fix highlight for HJSON (#208858)\n\n## Summary\r\n\r\nThis PR fixes the
missing highlight theme for HJSON specs in Vega.\r\nThe issue
#205711 was caused
by\r\nhttps://github.com//pull/182348 that
inadvertently\r\nspecified the code-editor package as sideEffect free.
This cause the\r\ncompiler to exclude every import without exported and
used methods.\r\nThis was the case for the code-editor that registered
some language\r\nhighlighters in that way.\r\n\r\nThe solution adopted
here is to mark the register_language.ts file as a\r\nfile with side
effects, the alternative solution can be to register\r\nthese directly
from within the code_editor component.\r\nA third option is to move
these registration within the monaco package\r\nwhere other languages
are also registered.\r\nI'd like to leave to @elastic/appex-sharedux the
preference to followup\r\nwith a better fix for the future.\r\n\r\nI'd
also like to have the opinion from @elastic/kibana-operations\r\nbecause
the misconfigured package issue can be seen only in production\r\nand
not in development mode. Is it possible that webpack doesn't
apply\r\ntreeshaking when in development mode?\r\n\r\nfix
https://github.com/elastic/kibana/issues/205711\r\nShould also fix the
same issue but for TSVB Markdown\r\n\r\nThe fix was tested on CI by
running at first only the CI FT with the\r\n`sideEffects:false` to
verify the failure
(see\r\n[build](https://buildkite.com/elastic/kibana-pull-request/builds/272375))\r\nand
then with the fix specifying the actual file that contains
side\r\neffects.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/208858","number":208858,"mergeCommit":{"message":"[Vega]
Fix highlight for HJSON (#208858)\n\n## Summary\r\n\r\nThis PR fixes the
missing highlight theme for HJSON specs in Vega.\r\nThe issue
#205711 was caused
by\r\nhttps://github.com//pull/182348 that
inadvertently\r\nspecified the code-editor package as sideEffect free.
This cause the\r\ncompiler to exclude every import without exported and
used methods.\r\nThis was the case for the code-editor that registered
some language\r\nhighlighters in that way.\r\n\r\nThe solution adopted
here is to mark the register_language.ts file as a\r\nfile with side
effects, the alternative solution can be to register\r\nthese directly
from within the code_editor component.\r\nA third option is to move
these registration within the monaco package\r\nwhere other languages
are also registered.\r\nI'd like to leave to @elastic/appex-sharedux the
preference to followup\r\nwith a better fix for the future.\r\n\r\nI'd
also like to have the opinion from @elastic/kibana-operations\r\nbecause
the misconfigured package issue can be seen only in production\r\nand
not in development mode. Is it possible that webpack doesn't
apply\r\ntreeshaking when in development mode?\r\n\r\nfix
https://github.com/elastic/kibana/issues/205711\r\nShould also fix the
same issue but for TSVB Markdown\r\n\r\nThe fix was tested on CI by
running at first only the CI FT with the\r\n`sideEffects:false` to
verify the failure
(see\r\n[build](https://buildkite.com/elastic/kibana-pull-request/builds/272375))\r\nand
then with the fix specifying the actual file that contains
side\r\neffects.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4"}}]}]
BACKPORT-->

Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
  • Loading branch information
kibanamachine and markov00 authored Feb 3, 2025
1 parent f166680 commit 1ebdc33
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
6 changes: 4 additions & 2 deletions packages/shared-ux/code_editor/impl/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@
"private": true,
"version": "1.0.0",
"license": "Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0",
"sideEffects": false
}
"sideEffects": [
"./register_languages.ts"
]
}
10 changes: 10 additions & 0 deletions test/functional/apps/visualize/group6/_vega_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

describe('vega chart', () => {
it('code-editor correct syntax highlight langs', async () => {
const hasRequiredLanguages = await browser.execute(() => {
const langs: Array<{ id: string }> =
// @ts-ignore
window.MonacoEnvironment?.monaco?.languages?.getLanguages() ?? [];
return langs.some((l) => l?.id === 'hjson') && langs.some((l) => l?.id === 'xjson');
});
expect(hasRequiredLanguages).to.be(true);
});

describe('initial render', () => {
it('should have some initial vega spec text', async function () {
const vegaSpec = await vegaChart.getSpec();
Expand Down

0 comments on commit 1ebdc33

Please sign in to comment.