[Vega] Fix highlight for HJSON #208858
Merged
[Vega] Fix highlight for HJSON #208858
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markov00
added
release_note:fix
backport:prev-major
💚 Build Succeeded
Metrics [docs]Page load bundle
History
|
|
Pinging @elastic/kibana-visualizations (Team:Visualizations) |
Dosant
approved these changes
Feb 3, 2025
There was a problem hiding this comment.
great find 👏
A third option is to move these registration within the monaco package where other languages are also registered.
I am really not sure why we ended up with languages in two places. not sure if there is any benefit. @eokoneyo, maybe you'd have an idea? Otherwise, I'd indeed opt for consolidating.
@Dosant Eyo is already working on this #208950, but for the sake of the fix, this is a first step toward that goal and, at lease for the test point of view , it could prevent regressions |
|
Starting backport for target branches: 8.16, 8.17, 8.18, 8.x |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 3, 2025
## Summary This PR fixes the missing highlight theme for HJSON specs in Vega. The issue elastic#205711 was caused by elastic#182348 that inadvertently specified the code-editor package as sideEffect free. This cause the compiler to exclude every import without exported and used methods. This was the case for the code-editor that registered some language highlighters in that way. The solution adopted here is to mark the register_language.ts file as a file with side effects, the alternative solution can be to register these directly from within the code_editor component. A third option is to move these registration within the monaco package where other languages are also registered. I'd like to leave to @elastic/appex-sharedux the preference to followup with a better fix for the future. I'd also like to have the opinion from @elastic/kibana-operations because the misconfigured package issue can be seen only in production and not in development mode. Is it possible that webpack doesn't apply treeshaking when in development mode? fix elastic#205711 Should also fix the same issue but for TSVB Markdown The fix was tested on CI by running at first only the CI FT with the `sideEffects:false` to verify the failure (see [build](https://buildkite.com/elastic/kibana-pull-request/builds/272375)) and then with the fix specifying the actual file that contains side effects. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 7260564)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 3, 2025
## Summary This PR fixes the missing highlight theme for HJSON specs in Vega. The issue elastic#205711 was caused by elastic#182348 that inadvertently specified the code-editor package as sideEffect free. This cause the compiler to exclude every import without exported and used methods. This was the case for the code-editor that registered some language highlighters in that way. The solution adopted here is to mark the register_language.ts file as a file with side effects, the alternative solution can be to register these directly from within the code_editor component. A third option is to move these registration within the monaco package where other languages are also registered. I'd like to leave to @elastic/appex-sharedux the preference to followup with a better fix for the future. I'd also like to have the opinion from @elastic/kibana-operations because the misconfigured package issue can be seen only in production and not in development mode. Is it possible that webpack doesn't apply treeshaking when in development mode? fix elastic#205711 Should also fix the same issue but for TSVB Markdown The fix was tested on CI by running at first only the CI FT with the `sideEffects:false` to verify the failure (see [build](https://buildkite.com/elastic/kibana-pull-request/builds/272375)) and then with the fix specifying the actual file that contains side effects. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 7260564)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 3, 2025
## Summary This PR fixes the missing highlight theme for HJSON specs in Vega. The issue elastic#205711 was caused by elastic#182348 that inadvertently specified the code-editor package as sideEffect free. This cause the compiler to exclude every import without exported and used methods. This was the case for the code-editor that registered some language highlighters in that way. The solution adopted here is to mark the register_language.ts file as a file with side effects, the alternative solution can be to register these directly from within the code_editor component. A third option is to move these registration within the monaco package where other languages are also registered. I'd like to leave to @elastic/appex-sharedux the preference to followup with a better fix for the future. I'd also like to have the opinion from @elastic/kibana-operations because the misconfigured package issue can be seen only in production and not in development mode. Is it possible that webpack doesn't apply treeshaking when in development mode? fix elastic#205711 Should also fix the same issue but for TSVB Markdown The fix was tested on CI by running at first only the CI FT with the `sideEffects:false` to verify the failure (see [build](https://buildkite.com/elastic/kibana-pull-request/builds/272375)) and then with the fix specifying the actual file that contains side effects. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 7260564)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 3, 2025
## Summary This PR fixes the missing highlight theme for HJSON specs in Vega. The issue elastic#205711 was caused by elastic#182348 that inadvertently specified the code-editor package as sideEffect free. This cause the compiler to exclude every import without exported and used methods. This was the case for the code-editor that registered some language highlighters in that way. The solution adopted here is to mark the register_language.ts file as a file with side effects, the alternative solution can be to register these directly from within the code_editor component. A third option is to move these registration within the monaco package where other languages are also registered. I'd like to leave to @elastic/appex-sharedux the preference to followup with a better fix for the future. I'd also like to have the opinion from @elastic/kibana-operations because the misconfigured package issue can be seen only in production and not in development mode. Is it possible that webpack doesn't apply treeshaking when in development mode? fix elastic#205711 Should also fix the same issue but for TSVB Markdown The fix was tested on CI by running at first only the CI FT with the `sideEffects:false` to verify the failure (see [build](https://buildkite.com/elastic/kibana-pull-request/builds/272375)) and then with the fix specifying the actual file that contains side effects. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 7260564)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Feb 3, 2025
# Backport This will backport the following commits from `main` to `8.18`: - [[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":"[email protected]"},"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 <[email protected]>","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 <[email protected]>","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 <[email protected]>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <[email protected]>
kibanamachine
added a commit
that referenced
this pull request
Feb 3, 2025
# 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":"[email protected]"},"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 <[email protected]>","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 <[email protected]>","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 <[email protected]>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <[email protected]>
kibanamachine
added a commit
that referenced
this pull request
Feb 3, 2025
# Backport This will backport the following commits from `main` to `8.x`: - [[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":"[email protected]"},"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 <[email protected]>","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 <[email protected]>","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 <[email protected]>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <[email protected]>
4 tasks
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
kibanamachine
added
the
backport missing
kibanamachine
added a commit
that referenced
this pull request
Feb 5, 2025
# Backport This will backport the following commits from `main` to `8.16`: - [[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":"[email protected]"},"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 <[email protected]>","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 <[email protected]>","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 <[email protected]>","sha":"7260564f6a9cd7da71a80e9bd57d836a0bea57c4"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <[email protected]>
kibanamachine
added
v8.16.4
and removed
backport missing
eokoneyo
added a commit
that referenced
this pull request
Feb 18, 2025
…onaco package (#208950) ## Summary This PR was created in response to #208858, it migrates all existing language definitions within the `@kbn/code-editor` package into the `@kbn/monaco` package to provide a separation of concern for logic that doesn't particularly relate to the configurations for the code editor UI. With this change, all supported languages are ingested from the `@kbn/monaco` package where they will be domiciled from henceforth, and in turn fix the issue that was discovered relating to the way the language definitions within `@kbn/code-editor` get registered as a side effect. With this change, to add support for a new language, said language should be defined within `@kbn/monaco`, and registered in `languages/index.ts`. <!-- ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --> --------- Co-authored-by: kibanamachine <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the missing highlight theme for HJSON specs in Vega.
The issue #205711 was caused by #182348 that inadvertently specified the code-editor package as sideEffect free. This cause the compiler to exclude every import without exported and used methods.
This was the case for the code-editor that registered some language highlighters in that way.
The solution adopted here is to mark the register_language.ts file as a file with side effects, the alternative solution can be to register these directly from within the code_editor component.
A third option is to move these registration within the monaco package where other languages are also registered.
I'd like to leave to @elastic/appex-sharedux the preference to followup with a better fix for the future.
I'd also like to have the opinion from @elastic/kibana-operations because the misconfigured package issue can be seen only in production and not in development mode. Is it possible that webpack doesn't apply treeshaking when in development mode?
fix #205711
Should also fix the same issue but for TSVB Markdown
The fix was tested on CI by running at first only the CI FT with the
sideEffects:falseto verify the failure (see build) and then with the fix specifying the actual file that contains side effects.