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

Models with "@" in their name do not resolve as dependencies #2295

Closed
lukasbach opened this issue Jan 2, 2021 · 10 comments · Fixed by #3057
Closed

Models with "@" in their name do not resolve as dependencies #2295

lukasbach opened this issue Jan 2, 2021 · 10 comments · Fixed by #3057
Labels
bug Issue identified by VS Code Team member as probable bug typescript-multifile

Comments

@lukasbach
Copy link

The following code examples can be copied into https://microsoft.github.io/monaco-editor/playground.html for verification.

I'm trying to specify dependencies manually via monaco.editor.createModel(), which works fine for dependency outside of organizations, but if a package is defined within a organization, i.e. has a name like @org/packagename and thus contains a @-character, it breaks. To me it looks like this is due to Monaco changing the @-character to %40.

The following code (which does not contain @) works fine:

monaco.editor.create(document.getElementById("container"), {
    model: monaco.editor.createModel(
        "import * as t from 'test';\nt.X.toExponential();\nt.Y; // Error", 
        "typescript",
        monaco.Uri.parse("file://root/test.ts")
    )
});

monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
  ...monaco.languages.typescript.typescriptDefaults.getCompilerOptions(),
  moduleResolution: monaco.languages.typescript.ModuleResolutionKind.NodeJs
});

monaco.editor.createModel(
    JSON.stringify({
        name: "test", version: "1.0.0",
        typings: "./lib/index.d.ts"
    }), 
    "typescript",
    monaco.Uri.parse("file://root/node_modules/test/package.json")
);

monaco.editor.createModel(
    "export const X = 1;", 
    "typescript",
    monaco.Uri.parse("file://root/node_modules/test/lib/index.d.ts")
);

console.log(
    "Works:",
    monaco.editor.getModel("file://root/node_modules/test/lib/index.d.ts").uri.toString()
);

However, on this code (containing a @) it breaks;

monaco.editor.create(document.getElementById("container"), {
    model: monaco.editor.createModel(
        "import * as t from '@org/test';\nt.X.toExponential();\nt.Y; // Error", 
        "typescript",
        monaco.Uri.parse("file://root/test.ts")
    )
});

monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
  ...monaco.languages.typescript.typescriptDefaults.getCompilerOptions(),
  moduleResolution: monaco.languages.typescript.ModuleResolutionKind.NodeJs
});

monaco.editor.createModel(
    JSON.stringify({
        name: "@org/test", version: "1.0.0",
        typings: "./lib/index.d.ts"
    }), 
    "typescript",
    monaco.Uri.parse("file://root/node_modules/@org/test/package.json")
);

monaco.editor.createModel(
    "export const X = 1;", 
    "typescript",
    monaco.Uri.parse("file://root/node_modules/@org/test/lib/index.d.ts")
);

console.log(
    "Does not work, but should:",
    monaco.editor.getModel("file://root/node_modules/@org/test/lib/index.d.ts")?.uri.toString()
);

console.log(
    "Does work, but doesnt help:",
    monaco.editor.getModel("file://root/node_modules/%40org/test/lib/index.d.ts").uri.toString()
);

monaco-editor version: 0.21.2
Browser: Chrome
OS: Windows 10

Is this an issue with Monaco or with my approach? Is there a workaround for this problem? Let me know if you need more details.

@zerdos
Copy link

zerdos commented Jan 10, 2021

try to add them whithout the /lib/ subdir, so your index.d.ts whill work here:
file://root/node_modules/@org/test/index.d.ts

@zerdos
Copy link

zerdos commented Jan 10, 2021

also, I think, it should be
file:///...
instad of
file://

for example:
``` const importHelper = [
      {
        name: "react",
        url: "https://unpkg.com/@types/react@17.0.0/index.d.ts"
      },
      {
        name: "global",
        url: "https://unpkg.com/@types/react@17.0.0/global.d.ts"
      },
      {
        name: "prop-types",
        url: "https://unpkg.com/@types/prop-types@15.7.3/index.d.ts"
      },
      {
        name: "react-dom",
        url: "https://unpkg.com/@types/react-dom@17.0.0/index.d.ts"
      },
      {
        name: "csstype",
        url: "https://unpkg.com/csstype@3.0.6/index.d.ts"
      }
    ];
    const dts = importHelper.map(({ name, url }) =>
      (async () =>
       monaco.languages.typescript.typescriptDefaults.addExtraLib(
          await (await fetch(
            url,
          )).text(),
          name.includes("@")
            ? `file:///node_modules/${name}`
            : `file:///node_modules/@types/${name}/index.d.ts`,
        ))()
    );

@lukasbach
Copy link
Author

Hi @zerdos, thanks for your response and suggestions! Unfortunately, they did not work in the playground for me, I still get the same issue. As I've mentioned before, I suspect the issue lies somewhere around the fact that monaco changes the @-character to %40 when making the path accessible, which I am unsure on how to work around and why that happens in the first place.

@alexdima alexdima added typescript-multifile bug Issue identified by VS Code Team member as probable bug labels Jan 25, 2021
@ErkoKnoll
Copy link

ErkoKnoll commented Mar 8, 2021

I once had a similar issue. I can't find the original issue but the way to make scope packages to work is to remove @ from the path and replace first / with __. Following code should fix your paths:

const fixedPackageName = name.startsWith('@') ? name.slice(1).replace('/', '__') : name

And the full file path should be: file:///node_modules/@types/${fixedPackageName}/...

@nissoh
Copy link

nissoh commented Mar 8, 2021

I once had a similar issue. I can't find the original issue but the way to make scope packages to work is to remove @ from the path and replace first / with __. Following code should fix your paths:

const fixedPackageName = name.startsWith('@') ? name.slice(1).replace('/', '__') : name

And the full file path should be: file:///node_modules/@types/${fixedPackageName}/...

@ErkoKnoll i believe this might work using addExtraLib but will fail with editor.createModel

@SamVerschueren
Copy link

The problem here is that if you don't add it to your models, you will not be able to peek the definition. So for that, you need to add it to both the extra lib and the model.

So after debugging this issue for half a day and trying to come up with a solution, which I didn't, I found out that the problem can basically be tracked down to this method.

image

So it tries to remove the @types prefix, which it fails to do here because the @ is encoded. I then tried to modify that code so it also strips off %40types/, but then I ended up with two suggestions.

image

So I basically gave up on this and instead I do not add the @types type definitions to the models. So peek definition and what not works for all unscoped packages but not for scoped packages unfortunately.

@lstkz
Copy link

lstkz commented Sep 8, 2021

I am also having this issue.
code

  editor.createModel(
      source,
      'typescript',
      this.monaco.Uri.parse(`file:///node_modules/@reduxjs/toolkit/index.d.ts`)
    )
// this works
import { createSlice } from '%40reduxjs/toolkit';

// this gives an error
import { createSlice } from '@reduxjs/toolkit';

It's a big deal for me because code written in my editor won't be the same as code written in native VSCode.

@lstkz
Copy link

lstkz commented Sep 9, 2021

A full example that can be pasted in https://microsoft.github.io/monaco-editor/playground.html

monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
    target: monaco.languages.typescript.ScriptTarget.Latest,
    allowNonTsExtensions: true,
    moduleResolution: monaco.languages.typescript.ModuleResolutionKind.NodeJs,
    module: monaco.languages.typescript.ModuleKind.ESNext,
    noEmit: true,
    esModuleInterop: true,
    strict: true,
    jsx: monaco.languages.typescript.JsxEmit.React,
    reactNamespace: 'React',
    allowJs: true,
    isolatedModules: true, 
})

monaco.languages.typescript.typescriptDefaults.setEagerModelSync(true);

const instance = monaco.editor.create(document.getElementById("container"), {
    language: 'typescript', 
    model: null,   
});

monaco.editor.createModel(
    'export const foo = {bar: 123};',
    'typescript',
    this.monaco.Uri.parse(`file:///node_modules/@reduxjs/toolkit/index.d.ts`)
)


const model = monaco.editor.createModel(
    `import {foo} from "@reduxjs/toolkit"

console.log(foo.bar);
`,
    'typescript',
    this.monaco.Uri.parse(`file:///index.ts`)
)

instance.setModel(model)

image

Changing @ -> %40 works

image

cc @orta

@eye-horus
Copy link

Any update on this? I also have the same issue

bsorrentino added a commit to bsorrentino/monaco-editor that referenced this issue Apr 4, 2022
component: TypeScriptWorker (tsWorker.ts)
- Enable 'skipEncoding' flag on Uri.toString invokation on 
getScriptFileNames() method
- Compare 'fileName' argument provided to _getModel() method both with 
Uri encoded and not
@bsorrentino
Copy link
Contributor

Hi

I've worked trying to understand the problem and finally I found out.

I've submitted PR #3057

hediet added a commit that referenced this issue Aug 3, 2022
Fix issue #2295 - Models with "@" in their name do not resolve as dependencies
@github-actions github-actions bot locked and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug typescript-multifile
Projects
None yet
9 participants