Allow to init and start separately.

[kaisalmen]

This allow to init and start the wrapper in two steps (from #56).
Using just start is still considered the default.
This changes enforces to dispose otherwise init will throw an exception.

This is integrated in the react component as well now. Most method are now protected, so a derived component can override the handleReinit, destroyMonaco, initMonaco or startMonaco.

• isReInitRequired was extracted from initMonaco and made protected to allow overriding it.

[kaisalmen]

New next versions are available:
https://www.npmjs.com/package/monaco-editor-wrapper/v/3.4.0-next.2
https://www.npmjs.com/package/@typefox/monaco-editor-react/v/2.4.0-next.2

These version use monaco-languageclient@7.0.0 and therefore obey: https://github.com/TypeFox/monaco-languageclient#new-with-v7-treemended-monaco-editor

[kaisalmen]

@cdietrich your opinion or review of the react component changes would be helpful. 🙂

[cdietrich]

will need to when when i find time to look. maybe tomorrow

[kaisalmen]

will need to when when i find time to look. maybe tomorrow

No rush, sounds good. 👍

[cdietrich]

in a first step i just bumped. this gives

[vite]: Rollup failed to resolve import "monaco-editor/esm/vs/platform/remote/common/remoteHosts.js" from "/Users/dietrich/git/my-monaco-editor-react-example/node_modules/vscode/vscode/src/vs/platform/telemetry/common/telemetryUtils.js".

any idea?

[cdietrich]

npm run dev gives

 Could not resolve "monaco-editor/esm/vs/platform/userDataProfile/common/userDataProfile.js"

    node_modules/@codingame/monaco-vscode-configuration-service-override/configuration.js:14:41:
      14 │ ...serDataProfilesService } from 'monaco-editor/esm/vs/platform/userDataProfile/common/userDataProfile.js';
         ╵                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.....
  You can mark the path "monaco-editor/esm/vs/platform/userDataProfile/common/userDataProfile.js" as
  external to exclude it from the bundle, which will remove this error.

✘ [ERROR] Could not resolve "monaco-editor/esm/vs/platform/remote/common/remoteHosts.js"

...

[kaisalmen]

@cdietrich did you check this (from second comment):

These version use monaco-languageclient@7.0.0 and therefore obey: https://github.com/TypeFox/monaco-languageclient#new-with-v7-treemended-monaco-editor

[cdietrich]

i have these in my package.json.
shouldnt that fullfill it

  "dependencies": {
    "@codingame/monaco-vscode-configuration-service-override": "^1.83.5",
    "@codingame/monaco-vscode-editor-service-override": "^1.83.5",
    "@codingame/monaco-vscode-files-service-override": "^1.83.5",
    "@typefox/monaco-editor-react": "^2.4.0-next.2",
    "monaco-editor-workers": "^0.44.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "vscode": "npm:@codingame/monaco-vscode-api@^1.83.5"
  },

[kaisalmen]

Now you need to define the overrides for any sub-dependencies to be inline. Just do it as done here:
https://github.com/TypeFox/monaco-components/blob/main/packages/examples/package.json

resolutions is only for yarn. overrides work with npm/pnpm

[cdietrich]

ok, is there an easy way to find out which service package has which missing dependency

[cdietrich]

just adding the overrides and resolutions does not help
see https://github.com/cdietrich/my-monaco-editor-react-example/compare/cd/700test

[kaisalmen]

"monaco-editor": "$monaco-editor" is a reference to a defined dependency. You have a defined dependency for vscode but not for monaco-editor. Either fully specify in overrides or define it in dependencies:
"monaco-editor": "npm:@codingame/monaco-editor-treemended@>=1.83.5 <1.84.0",

When I do that it works

[kaisalmen]

ok, is there an easy way to find out which service package has which missing dependency

missing is not the problem. mismatch versions are the problem. Sorry, this produces another hick-up, but the other approach was worse, because it the patching/treemending monaco-editor was hidden from the user/project. Now it is explicitly required/stated

[cdietrich]

problem is that it still does not work.
unfortunately i am still and noob in the npm ecosystem so i have. no clue what is happening

[kaisalmen]

unfortunately i am still and noob in the npm ecosystem so i have. no clue what is happening

It is a mess. I am moving between noob or expert every day. 😆

Remove everything from your checkout (git clean -f -X -d) and do it again. That is my only change.

diff --git a/package.json b/package.json
index 3bee246..4494b8d 100644
--- a/package.json
+++ b/package.json
@@ -15,6 +15,7 @@
     "@codingame/monaco-vscode-files-service-override": "^1.83.5",
     "@typefox/monaco-editor-react": "^2.4.0-next.2",
     "monaco-editor-workers": "^0.44.0",
+    "monaco-editor": "npm:@codingame/monaco-editor-treemended@>=1.83.5 <1.84.0",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",
     "vscode": "npm:@codingame/monaco-vscode-api@^1.83.5"

[cdietrich]

yes -fdx helped me too.
thx. now i can start testing my actual stuff later today or 2morrow

[kaisalmen]

yes -fdx helped me too.

I add that as npm script in most projects now.

[cdietrich]

i wonder if we can make the
mustReInit
accessible.

or something else where i can hook in and have access to this.getEditorWrapper().getLanguageClient() (any maybe old props)
so that i can send messages to the server

maybe optional parameter on onLoad callback?

[kaisalmen]

i wonder if we can make the
mustReInit
accessible.

Good idea. I will do that

[cdietrich]

good with that i can check for changes of otherFileContent or otherFileUri here and my startMonaco override will be called. still need to find out how i access the old props in startMonaco so that i can close them. maybe can use some state to store it.
right now i workaround this by setting a key with date on parent component but of course this will lead to needless mounts

      <MonacoEditorReactCompExtended
    // key={new Date().toISOString() /* without this the updated monaco seems to not usable at all */}  
    userConfig={userConfig}
    onLoad={onLoad}
    otherFileContent={otherFileContent}
    otherFileUri={otherFileUri}
    style={{
      paddingTop: '5px',
      height: '40vh',
      width: '100%',
    }}
    
        />

[kaisalmen]

Pushed an update an new next versions:
https://www.npmjs.com/package/monaco-editor-wrapper/v/3.4.0-next.3
https://www.npmjs.com/package/@typefox/monaco-editor-react/v/2.4.0-next.3

[cdietrich]

cool that worked. as monaco will be started i dont need to close old docs

[cdietrich]

i still see something i dont understand.
why in my example

already returns true

[cdietrich]

improper compare of objects?

[kaisalmen]

@cdietrich can I reproduce this with your above repo?

[cdietrich]

Yea if you use the recently updated branch

[kaisalmen]

@cdietrich I can re-produce the issue with your example. The comparison only works properly for simple properties. That should be re-producible by a unit test (=fix template). Will do it with this PR, too. Thank your for spotting this.

[kaisalmen]

As you can see the test fails now ⬇️

[kaisalmen]

@cdietrich The comparison problem is resolved:
https://www.npmjs.com/package/monaco-editor-wrapper/v/3.4.0-next.4
https://www.npmjs.com/package/@typefox/monaco-editor-react/v/2.4.0-next.4

[cdietrich]

nice and thanks. works fine

[kaisalmen]

Latest status is available:
https://www.npmjs.com/package/monaco-editor-wrapper/v/3.4.0-next.5
https://www.npmjs.com/package/@typefox/monaco-editor-react/v/2.4.0-next.5

[montymxb]

Some general feedback. In particular I would be a fan of renaming start to initAndStart (or something similar to convey the two steps being a part of that). startNoInit can be just start then, which implicitly requires init to be run before.

Some function headers on the new methods would be helpful for other users as well, so that the intention of the new API is clearly outlined.

Local tests & double checking the examples look good by the way.

[montymxb]

One last thing, noInitJustStart can probably be start, and then the function header makes it clear that this doesn't init (or requires init to be invoked first).

[kaisalmen]

Thank you @montymxb I merged it after performing the latest changes.
https://www.npmjs.com/package/monaco-editor-wrapper/v/3.4.0-next.7
https://www.npmjs.com/package/@typefox/monaco-editor-react/v/2.4.0-next.7

I want to included the latest @codingame/minaco-vscode-api before releasing new final versions.