Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does it make sense to switch to monaco-editor instead of monaco-editor-core #337

Closed
kaisalmen opened this issue Mar 24, 2022 · 8 comments
Closed

Comments

@kaisalmen
Copy link
Collaborator

kaisalmen commented Mar 24, 2022

I was wondering if we should switch to the dependency of monaco-editor-core to monaco-editor.

Why? Because you cannot import both monaco-editor and monaco-editor-core in the same project/app. This blocks using features available from the main editor (like js support) and adding a new language + LS connection via this project, because both define the same things in global space.

As far as I understand it you can just important the "core" feature of monaco-editor like this:

import * as monaco from 'monaco-editor/esm/vs/editor/editor.api';

and then you achieve the same as now, but you don't block the combined usage outlined above. Is this reasoning correct? WDYT? Does this have any bad implications I am overlooking?

@CGNonofr
Copy link
Collaborator

I agree!

Btw, since we only import it for types, It won't change anything to the generated code, will it?

We can switch on monaco-editor for the example tho, we'll at least have the syntax highlighting.

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Mar 24, 2022

Btw, since we only import it for types, It won't change anything to the generated code, will it?

It should be the same, but I would first compare what results both import/dependency combinations provide (hope this makes sense 🙂)

An application (or in the example) can then selectively import what is needed from the possible things/features:

import 'monaco-editor/esm/vs/editor/editor.all.js';

// select features
import 'monaco-editor/esm/vs/editor/standalone/browser/accessibilityHelp/accessibilityHelp.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/inspectTokens/inspectTokens.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/iPadShowKeyboard/iPadShowKeyboard.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/quickAccess/standaloneHelpQuickAccess.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/quickAccess/standaloneGotoLineQuickAccess.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/quickAccess/standaloneGotoSymbolQuickAccess.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/quickAccess/standaloneCommandsQuickAccess.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/quickInput/standaloneQuickInputService.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/referenceSearch/standaloneReferenceSearch.js';
import 'monaco-editor/esm/vs/editor/standalone/browser/toggleHighContrast/toggleHighContrast.js';

// add workers
import 'monaco-editor/esm/vs/language/typescript/monaco.contribution';
import 'monaco-editor/esm/vs/language/html/monaco.contribution';
import 'monaco-editor/esm/vs/language/css/monaco.contribution';
import 'monaco-editor/esm/vs/language/json/monaco.contribution';

// support all basic-languages
import 'monaco-editor/esm/vs/basic-languages/monaco.contribution';

import * as monaco from 'monaco-editor/esm/vs/editor/editor.api';

@CGNonofr
Copy link
Collaborator

The javascript won't change, but the types will changes indeed. I don't see a single reason why someone would use monaco-editor-core instead of monaco-editor

Btw importing monaco-editor/esm/vs/editor/editor.api instead of monaco-editor makes no difference here.

What is a little weird however is that monaco-editor or monaco-editor-core is not in the dependencies of monaco-languageclient (not event in peer dependencies)

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Mar 24, 2022

What is a little weird however is that monaco-editor or monaco-editor-core is not in the dependencies of monaco-languageclient (not event in peer dependencies)

This sounds fishy. So, this only works because the example imports monaco-editor core in the node-modules or is it something like a transient dependency that also ends up in the node-modules folder?

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Mar 24, 2022

Otherwise, the TypeScript compile step must go 💥 because the import does not exist!? Ok, I will address this in #331 as well.

@CGNonofr
Copy link
Collaborator

CGNonofr commented Mar 24, 2022

It only works because at the only place we need to have access to the monaco namespace are when we install the services, and we expect to developer to provide the monaco namespace as parameter, so they can import it any way they want, it can be either monaco-editor or monaco-editor-core.

The only issue with it is that we reference monaco-editor-core in the d.ts files and it's probably not installed so typescript infer it's any

Otherwise, the TypeScript compile step must go boom because the import does not exist!? Ok, I will address this in #331 as well.

I needed to enable skipLibCheck for this reason IIRC

@kaisalmen
Copy link
Collaborator Author

so ther can import it any way they want, it can be either monaco-editor or monaco-editor-core.

💡

I needed to enable skipLibCheck for this reason IIRC

Ok, also good to know.

@kaisalmen
Copy link
Collaborator Author

Resolved with #340

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

No branches or pull requests

2 participants