monaco-editor dependency with a more conservative semver range.
#45133
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
jloleysens
added
Team:Presentation
|
Pinging @elastic/es-ui |
|
Pinging @elastic/kibana-canvas |
|
Pinging @elastic/code |
💔 Build Failed |
jloleysens
force-pushed
the
lock_monaco_editor
branch
from
665e23a
to
e67d03d
Compare
💚 Build Succeeded |
cjcenizal
approved these changes
Sep 10, 2019
There was a problem hiding this comment.
The reasoning behind this change makes sense to me and the changes themselves look good. I think we should be proactive about bringing this to the Code team and requiring their approval before merging though, since AFAIK they're the primary consumers of Monaco right now.
| @@ -78,6 +78,7 @@ | |||
| }, | |||
| "resolutions": { | |||
| "**/@types/node": "10.12.27", | |||
| "**/@types/react": "16.8.3", | |||
There was a problem hiding this comment.
Could you explain this change for me?
There was a problem hiding this comment.
Sure, I added this to the description:
[UPDATE]
Using selective resolution for @types/react due to version specified in react-monaco-editor being broad
mw-ding
approved these changes
Sep 11, 2019
…ditor * 'master' of github.com:elastic/kibana: (76 commits) Upgrade EUI to 13.8.1 (elastic#45052) [ML] Add multi metric job wizard test (elastic#45279) [SIEM] Inject/apply KQL changed in refresh button (elastic#45065) [Graph] Type persistence (elastic#44985) Functional tests: convert more test/services to TS (elastic#45176) [ML] Fixes display of matching modules in index data visualizer (elastic#45261) [Console] Update indentation behaviour (elastic#45249) Convert value provided to PhraseValueInput to string to catch Exception (elastic#45259) [Region Map] Fix loading default vector map and base layer setting (elastic#43858) [ML] Fixing empty time range when cloning jobs (elastic#45286) [ML] Fixing wizard validation delay (elastic#45265) [Logs UI] Interpret finished analysis jobs as healthy (elastic#45268) [Console] SQL template with triple quote in completion (elastic#45248) [ML] Data Frames: Cards as links (elastic#45254) fix(code/frontend): should show updating instead of cloning when updating (elastic#45238) fix(code/frontend): fix document search result from (elastic#45236) disable another flaky suite (elastic#45323) (elastic#45330) disable flaky suite (elastic#45105) skip flaky suite (elastic#43069) skip flaky suite (elastic#45089) ...
💚 Build Succeeded |
poffdeluxe
approved these changes
Sep 11, 2019
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Sep 11, 2019
jloleysens
added a commit
that referenced
this pull request
Sep 12, 2019
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
TL;DR perhaps we should use
~instead of^for monaco-editor SemVer range.After playing around with Monaco Editor in Console I installed the latest version in the root
package.jsonas a dependency. I then encountered this bug for0.18.0. I know for the most part we are safe becauseyarn.locklocks versions into place despite the SemVer range being specified in package.json but in the case ofmonaco-editorit may be better to specify a slightly more conservative SemVer range because it's not yet a full major release.As an aside, later, when monaco is also used in not-x-pack code, we can move the dependency to just the root
package.jsonso that we are not potentially using multiple versions of it?[UPDATE]
Using selective resolution for @types/react due to version specified in
react-monaco-editorbeing broad.