Skip to content

Allow for lazy loading of monaco - #19

Merged
gatesn merged 1 commit into
TypeFox:masterfrom
gatesn:ngates/lazy-monaco-refs
Aug 28, 2017
Merged

Allow for lazy loading of monaco#19
gatesn merged 1 commit into
TypeFox:masterfrom
gatesn:ngates/lazy-monaco-refs

Conversation

@gatesn

@gatesn gatesn commented Aug 9, 2017

Copy link
Copy Markdown
Contributor

It's fine to import X = monaco.X for types, but not for objects such as monaco.Uri and monaco.languages. This PR inlines these references to allow monaco to be loaded lazily.

Fixes #16

@akosyakov

akosyakov commented Aug 10, 2017

Copy link
Copy Markdown
Contributor

@gatesn please sign off your commit, git commit --amend -s

Does this PR solve an issue with react-monaco-editor integration? Would you be able to add react-example in the root?

@gatesn
gatesn force-pushed the ngates/lazy-monaco-refs branch from 807a721 to c40b409 Compare August 10, 2017 15:25
@gatesn

gatesn commented Aug 10, 2017

Copy link
Copy Markdown
Contributor Author

Signed commit, will check out the react issue as well

@gatesn

gatesn commented Aug 11, 2017

Copy link
Copy Markdown
Contributor Author

Not sure it's worth writing an example here given the amount of duplicated code between the react and non-react examples. Also #16 is already closed.

@akosyakov

Copy link
Copy Markdown
Contributor

Would it be possible to add a react page to the existing example?

@gatesn

gatesn commented Aug 14, 2017

Copy link
Copy Markdown
Contributor Author

@akosyakov working on the example, I try to compile it and get this, have you seen it before? Is it an update in the jsonrpc package?


src/client/client.ts(43,18): error TS2554: Expected 1 arguments, but got 0.
src/client/client.ts(60,57): error TS2345: Argument of type 'MessageConnection' is not assignable to parameter of type 'MessageConnection'.
  Types of property 'sendRequest' are incompatible.
    Type '{ <R, E, RO>(type: RequestType0<R, E, RO>, token?: CancellationToken | undefined): Thenable<R>; <...' is not assignable to type '{ <R, E, RO>(type: RequestType0<R, E, RO>, token?: CancellationToken | undefined): Thenable<R>; <...'. Two different types with this name exist, but they are unrelated.
      Types of parameters 'type' and 'type' are incompatible.
        Type 'RequestType0<any, any, any>' is not assignable to type 'RequestType0<any, any, any>'. Two different types with this name exist, but they are unrelated.
          Types have separate declarations of a private property '_'.

@jkillian

Copy link
Copy Markdown

Nice fix! Can this be merged and the react-example added as a separate PR? They don't feel extremely related to me.

@akosyakov

Copy link
Copy Markdown
Contributor

@gatesn please use git rebase origin/master instead of merge to catch up with changes in the master

@akosyakov

Copy link
Copy Markdown
Contributor

@jkillian if you don't use react-monaco-editor, what does it fix for you?

@akosyakov
akosyakov self-requested a review August 25, 2017 09:36
@jkillian

Copy link
Copy Markdown

@akosyakov we use monaco's loader.js script to load monaco. If this library gets imported before monaco is loaded (which is very likely since for the most part we use static imports at the top of our files and then bundle with webpack), it'll throw an error since monaco isn't yet defined as a value.

@akosyakov

akosyakov commented Aug 25, 2017

Copy link
Copy Markdown
Contributor

@gatesn yes, with npm5 the example is broken because of symlinks used for file: and tsc was not able to preserve them leading to resolving MessageConnection from 2 different places

I've opened a PR to fix it #25, please review it.

@akosyakov akosyakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look at comments

@akosyakov

Copy link
Copy Markdown
Contributor

@gatesn please assign you on #4 (comment) if you are going to tackle the react example. I am fine to make it with the separate PR but please take care of the proper rebasing.

Signed-off-by: Nicholas Gates <ngates@palantir.com>
@gatesn
gatesn force-pushed the ngates/lazy-monaco-refs branch from 7d4b09d to 92fe66d Compare August 27, 2017 21:53
@akosyakov

Copy link
Copy Markdown
Contributor

@gatesn I've sent an invite you that makes you a collaborator. You should be able to merge after accepting it.

@gatesn
gatesn merged commit 4177f41 into TypeFox:master Aug 28, 2017
@gatesn
gatesn deleted the ngates/lazy-monaco-refs branch August 28, 2017 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants