Skip to content
This repository was archived by the owner on Nov 5, 2021. It is now read-only.

Conversation

@orta
Copy link
Contributor

@orta orta commented Jun 26, 2020

Previously: we bundled 2 DTS lib files and hardcoded the paths into the TS workers.

This PR:

  • The dts files are unbundled but not at the expense of DL size (1.2MB before, 1.2MB after)
  • The dts files are dropped into an object with their filename as key, and content as value in that object.
  • TS handles the look ups between objects via the <reference lib=' lines which are now back
  • The lookup for a dts file is changed in 2 significant ways:
    • The paths are only hardcoded for the same cases which are available today
    • The paths allow for looking inside the _extraLibs option to allow others (e.g. the playground) to add the rest of the DTS files at runtime


enqueue('');
enqueue('es6');
enqueue('es2015');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding the ES6 means that we can use the TS lib routing pattern 1-to-1, it adds 3-4 LOC (and then the copyright banner)

case 1:
case 0:
return "lib.d.ts";
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be future proof for ES2021 etc

@spahnke
Copy link
Contributor

spahnke commented Jun 26, 2020

Would this allow to load the ES libs but without the DOM APIs? I use the editor in a scripting environment where the DOM APIs are not present, except for one specific use case. So right now for that to work I have to set lib: [] in the compiler options and bundle the ES types myself and load them as an extra lib. Same for the DOM APIs, I add them as an extra lib for that one specific use case and remove them again afterwards. If this PR would enable this out of the box I would be really happy!

@orta
Copy link
Contributor Author

orta commented Jun 26, 2020

I've not tested trying without the DOM (I tested that it explicitly did, go figure) but I assume it would work that way because TypeScript works that way and all lib routing happens through it.

The PR also has an intentional way to let you set this up. You can override any DTS file via the extraLibs API - if you included a blank file in extraLibs at lib.dom.d.ts then that would take precedence in _getScriptText over the shipped dts file.

@orta
Copy link
Contributor Author

orta commented Jun 26, 2020

( Also this is running on the 4.0 playground: https://www.staging-typescript.org/play?ts=4.0.0-beta )

Co-authored-by: Wesley Wigham <[email protected]>
@orta
Copy link
Contributor Author

orta commented Jul 9, 2020

This PR got an update: the lib d.ts files are now represented as Monaco in-memory models. This allows for features like peek to work, and for people to reliably show the files in their Monaco instances.

@orta
Copy link
Contributor Author

orta commented Jul 20, 2020

I'd like to come look at this again at some point and make it so lib files don't type check, which can avoid the dancing in a213b30

@orta
Copy link
Contributor Author

orta commented Aug 6, 2020

When I checked Object.entries in a sandbox, it didn't show up - turned out my importing technique didn't bring in all the lib.d.ts files.

Now it does, this example is set to target: esnext:

Screen Shot 2020-08-06 at 1 41 57 PM

Doing this bumps the lib.d.ts from 1.2MB to 1.6MB. Removing webworker and esnext, can drop that down to 1.3mb.

@cefn
Copy link

cefn commented Aug 20, 2020

Followed to this PR from microsoft/TypeScript-Website#82

I am still experiencing...

Property 'entries' does not exist on type 'ObjectConstructor'.(2339)

...in both playground and staging playground, indicating Object.entries is missing within Typescript playground when choosing 4.0.0-beta and target esnext. Do we need to wait a while for this work to be stably deployed to the typescriptlang server?

https://www.staging-typescript.org/play?target=99&ts=4.0.0-beta#code/KYDwDg9gTgLgBAYwgOwM7wIbIJYFsMA2AClBCAJ5wBcAysBlAgBYlnkA8AgjvgQJL4A5sE5hsAPjgBeOAG8AUHDgAjDKmBUFSpQCIAjjqo6seQqh0AaRdp2psMYMgy5gh2wFcwwKMoB0SXEtrXQAzbAJgGHIvN18AKy9BOAB5ACU4XzBkJLSMwWwQnWsAXyslDDFNYLgTXirtbVUYKgBtaobZPSMQiIATb3x3VBT0gDdnMShXUvbtWTsHJxcjJtQQ6GBR738IQOLZgF0yhsQMZraTua6dBDORuABreyj7sCHUch0Zy6PZqDPWrMlJ0jP94LllNgkrgIENpodjkpvtpTMJ6id0OECIDLsCFo5nBoAOQ9bAIB5QHa4InIy6yMIRKJeKhE+JgHLpNnADkZLKCGkI2a1M7AXpUOAXXHzewE5ZE-JgJjkKk0xEnenhSLRYm+fIhAU-ar7JHHCrYZJQfpQVrGHiESw6VGuX7lMQAEWAIQw7gIMFQ6N0TrcwocvSKSPk+3kIXcyAQMGwKDgwhgdAYzAAqlACFw4KBFr1hgBpYDkUWibDsIsWABq4gscCLeZABeG6Cg2GyDZrzdbcHbncE4gAFGBSBRxWnGCxxxxOPW4J3RhBbgmUOK+Mhl6vE8gq7XxABKahwDOpAAyWjgjP79Gnm+3Z130jgY7YvlU6gA3NZ1lBh0gaDwC0DylhY4wEAccAQCEKTKHEwDxr4jgwB2wCoMOS4rk+KCHoeV5KCUkbyPITLAHA3CmAQL43GcOhwAAPnAOhgvRTE6E0Og-qR2pwAIGDCDRmIEAQbHMSGopcSRZFwFOzAAGLYMABC9DRBiMcx+JLK4Gk6AyWoxNxMkPtha57k2+aOIW-aoYO3a9lZww0LZ2SSDIV4gYuyCNgcAD84o1pG3Gdg4UBegg5FyUwJk7kmlnINZMU4XuUWKcpvQWAOrlyFGIXeOF5EVjl0m8SWZa9BWVYOQlbYuYItbVdZWVDi+HkPF5PkBhKowdTWByTnezBJWZLQHERuXIKFBUUXaBBFfF1kVhYZXlmI7BEsKBBEhYlG8JIsgTVNGARXxQgiGIjXDEtK0VWtRJOtt-HCOIB0kXlYXHYVs1Ped2CXTNVFLadAm-cV8jvdNUWsBQuYLVdYgvdYn4aFFw27tYZripwppiBaVrisOoHkDBFGHqNGPup63q+v6cjVJ5nZwITpYk5wh5+eKROsyBY0RsUQA

https://www.typescriptlang.org/play?target=99&ts=4.0.0-beta#code/KYDwDg9gTgLgBAYwgOwM7wIbIJYFsMA2AClBCAJ5wBcAysBlAgBYlnkA8AgjvgQJL4A5sE5hsAPjgBeOAG8AUHDgAjDKmBUFSpQCIAjjqo6seQqh0AaRdp2psMYMgy5gh2wFcwwKMoB0SXEtrXQAzbAJgGHIvN18AKy9BOAB5ACU4XzBkJLSMwWwQnWsAXyslDDFNYLgTXirtbVUYKgBtaobZPSMQiIATb3x3VBT0gDdnMShXUvbtWTsHJxcjJtQQ6GBR738IQOLZgF0yhsQMZraTua6dBDORuABreyj7sCHUch0Zy6PZqDPWrMlJ0jP94LllNgkrgIENpodjkpvtpTMJ6id0OECIDLsCFo5nBoAOQ9bAIB5QHa4InIy6yMIRKJeKhE+JgHLpNnADkZLKCGkI2a1M7AXpUOAXXHzewE5ZE-JgJjkKk0xEnenhSLRYm+fIhAU-ar7JHHCrYZJQfpQVrGHiESw6VGuX7lMQAEWAIQw7gIMFQ6N0TrcwocvSKSPk+3kIXcyAQMGwKDgwhgdAYzAAqlACFw4KBFr1hgBpYDkUWibDsIsWABq4gscCLeZABeG6Cg2GyDZrzdbcHbncE4gAFGBSBRxWnGCxxxxOPW4J3RhBbgmUOK+Mhl6vE8gq7XxABKahwDOpAAyWjgjP79Gnm+3Z130jgY7YvlU6gA3NZ1lBh0gaDwC0DylhY4wEAccAQCEKTKHEwDxr4jgwB2wCoMOS4rk+KCHoeV5KCUkbyPITLAHA3CmAQL43GcOhwAAPnAOhgvRTE6E0Og-qR2pwAIGDCDRmIEAQbHMSGopcSRZFwFOzAAGLYMABC9DRBiMcx+JLK4Gk6AyWoxNxMkPtha57k2+aOIW-aoYO3a9lZww0LZ2SSDIV4gYuyCNgcAD84o1pG3Gdg4UBegg5FyUwJk7kmlnINZMU4XuUWKcpvQWAOrlyFGIXeOF5EVjl0m8SWZa9BWVYOQlbYuYItbVdZWVDi+HkPF5PkBhKowdTWByTnezBJWZLQHERuXIKFBUUXaBBFfF1kVhYZXlmI7BEsKBBEhYlG8JIsgTVNGARXxQgiGIjXDEtK0VWtRJOtt-HCOIB0kXlYXHYVs1Ped2CXTNVFLadAm-cV8jvdNUWsBQuYLVdYgvdYn4aFFw27tYZripwppiBaVrisOoHkDBFGHqNGPup63q+v6cjVJ5nZwITpYk5wh5+eKROsyBY0RsUQA

@cefn
Copy link

cefn commented Aug 20, 2020

I can now see this functioning in the 4.0.2 version live on typescriptlang.org

@orta
Copy link
Contributor Author

orta commented Aug 21, 2020

Ah yeah, sorry for missing that - it would have worked on nightlies too!

@alexdima alexdima self-requested a review August 28, 2020 07:18
Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

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

Hey @orta I just pushed a couple of commits to simplify the code (without changing any of the import logic) and to address some stylistic issues.

But when I run npm run prepublishOnly on my machine, here are the file sizes I get:

  • [NOK] release/min/tsMode - 1.5M (from a previous size of 23.9K)
  • [OK] release/min/tsWorker - 3.8M (from a previous size of 3.3M)

This is caused by the new reference to lib/lib.ts in languageFeatures.ts. This ends up requiring the entire lib.d.ts contents on the UI thread.

@alexdima
Copy link
Member

I've modified the usages in languageFeatures.ts to work by fetching the lib.d.ts contents from the web worker as needed. This reduces again the minified tsMode.js to 28K.

@alexdima alexdima added this to the August 2020 milestone Aug 28, 2020
@alexdima alexdima merged commit 6d0fbd4 into master Aug 28, 2020
@alexdima alexdima deleted the let_ts_resolve_libs branch August 28, 2020 08:18
@orta
Copy link
Contributor Author

orta commented Aug 28, 2020

Hey @alexdima - thanks for the thorough look though

@spahnke
Copy link
Contributor

spahnke commented Sep 2, 2020

Would this allow to load the ES libs but without the DOM APIs? I use the editor in a scripting environment where the DOM APIs are not present, except for one specific use case. So right now for that to work I have to set lib: [] in the compiler options and bundle the ES types myself and load them as an extra lib. Same for the DOM APIs, I add them as an extra lib for that one specific use case and remove them again afterwards. If this PR would enable this out of the box I would be really happy!

Hi @orta, since this is now merged I tried running it out of sources to test my use case from above, but I'm running into some problems (or maybe I misunderstood this PR 😅 ). My instinct was that this PR enables the following code, where I can set lib to "esnext" so that just the ES types are included in the completion and not the DOM types. But when I do that I get no completion at all. I also tried your suggestion to use addExtraLib and pass it an emtpy lib.dom.d.ts (this time without the lib option of course) but curiously only the DOM completion shows up then. For instance, []. does not show any completions until I dispose that extra lib again, but you can see global completions for DOM types. Could you maybe help clear up my confusion? It's also perfectly fine if I just misunderstood this PR 😅 I can definitely see how this PR helps future proofing the TypeScript integration so thank you for that!

monaco.languages.typescript.javascriptDefaults.setCompilerOptions({
	target: monaco.languages.typescript.ScriptTarget.ESNext,
	lib: ["esnext"],
	allowNonTsExtensions: true
});

// monaco.languages.typescript.javascriptDefaults.addExtraLib("", "lib.dom.d.ts");

monaco.editor.create(document.getElementById("container"), {
	value: "",
	language: "javascript"
});

@orta
Copy link
Contributor Author

orta commented Sep 8, 2020

Interesting, maybe lib isn't quite working as I'd expect, without:

Screen Shot 2020-09-08 at 11 25 34 AM

with:

Screen Shot 2020-09-08 at 11 27 05 AM

But switching it to "lib":["lib.esnext.d.ts"], works. I think this is because TypeScript will normally do the mapping for esnext -> lib.esnext.d.ts for you behind the scenes, I don't think it'd hurt to have that happen in here by default. Will send a PR.

@igor-eremyashev
Copy link

@spahnke Hi! I'm having the exact same problem, addExtraLib("", "lib.dom.d.ts") disables hints from lib: ['lib.es2019.d.ts']. Were you able to resolve this issue?

@spahnke
Copy link
Contributor

spahnke commented Jan 19, 2021

My code from above now works in the current version of the editor (e.g. in the playground); the issue has been fixed with #68. You should be able to just use "es2019" instead of "esnext".

monaco.languages.typescript.javascriptDefaults.setCompilerOptions({
	target: monaco.languages.typescript.ScriptTarget.ESNext,
	lib: ["esnext"],
	allowNonTsExtensions: true
});

monaco.editor.create(document.getElementById("container"), {
	value: "",
	language: "javascript"
});

@igor-eremyashev
Copy link

igor-eremyashev commented Jan 19, 2021

I have the following code:

const typescript = monaco.languages.typescript;
            
typescript.javascriptDefaults.setCompilerOptions({
    lib: ['es2019'],
    target: typescript.ScriptTarget.ES2019,
    module: typescript.ModuleKind.ESNext,
    allowNonTsExtensions: true,
    scrollBeyondLastLine: false
});

//typescript.javascriptDefaults.addExtraLib("", "lib.dom.d.ts"); 

monaco.editor.create(document.getElementById('container'), {
	value: "",
	language: 'javascript'
});

When the line typescript.javascriptDefaults.addExtraLib("", "lib.dom.d.ts"); is commented out I get type hints for built-in methods as expected:
image
When I uncomment this line I start getting DOM type hints but all the type hints for built-in types no longer appear:
image
image
I need to be able to enable DOM type hints only in specific cases while having basic types as array or int available all the time.

@spahnke
Copy link
Contributor

spahnke commented Jan 19, 2021

Ah, you should be able to just do

monaco.languages.typescript.javascriptDefaults.setCompilerOptions({
	target: monaco.languages.typescript.ScriptTarget.ESNext,
	lib: ["es2019", "dom"],
	allowNonTsExtensions: true
});

instead of that line to empty the extra lib. That's what I use to switch between browser ("dom" and "esnext") and non-browser (just "esnext") contexts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants