-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Support additional libs via compiler API #3863
Conversation
Awesome! If you wanted to include an extra library in addition to Deno's declarations, would you need to include Also, I think that's the case but just to make sure, this PR also provides support for |
Actually,
Yes, though if you tried to |
b933e8b
to
47b022b
Compare
Hrm... the change in the thread pools I think has caused a bit of a problem. This has to resolve the remote module synchronously in TS land, though it is async in Rust. Need to figure out how to do that. |
Hrmmmm... I suspect I am going to need some help, because the debug log is showing that it is hanging while connecting to the remote server to fetch the remote lib file. I suspect that because of 161cf7c something has changed while trying to do main -> worker -> async op which is essentially blocking infinitely because there isn't a "free" thread:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kitsonk I'm trying to debug the problem, I'll get back when I find something.
@kitsonk I'm afraid the simplest/quickest/dirtiest solution would be to use reqwest's sync |
@bartlomieju well, it is wrapped up in the whole of the file_fetcher API to take advantage of the caching and the like, so using sync I will take a look at putting |
std/manual.md
Outdated
@@ -174,6 +174,12 @@ command line: | |||
$ deno types | |||
``` | |||
|
|||
The file is the concatenation of three library files that are built into Deno: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/file/output, remove previous link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old link is still there.
@kitsonk just use |
47b022b
to
1883dab
Compare
Actually, once I rebased on master and the changes seem to have unblocked the thread. So hoping the CI works now, as it is working locally. |
Crap, no, the file was in my cache, and I forgot |
@bartlomieju that effectively didn't work... I got something like this when using spawn_thread:
I also tried several variations of I've cleaned up the code in the op a little bit, and it is a bit more clear, but still get stuck at the same point. |
@kitsonk here's a hacky patch that will get you forward:
It spawns new thread for each remote assets - but I guess we can get away with it for a while. Like I said earlier I'll be refactoring file fetcher for path handling so I can fix this problem as well. |
2761553
to
c9ed8d7
Compare
Phew, finally this works! ping @ry |
d0c6134
to
d905a6f
Compare
Let's see what it look like if we build it in. If we require the full URL, then that is even more TypeScript code that will break under tsc, which doesn't do us any favours. |
Fixes denoland#3726 This PR provides support for referencing other lib files that are not used by default in Deno via the compiler APIs.
d905a6f
to
2b111e6
Compare
any idea what the delta on size of target/release/deno is? |
This PR:
Master:
I tried the include-flate crate and it works, but the macro only accepts string literals and a lot of our files need Most of the assets (including |
This is painful, but I think it's better than downloading at runtime. I'll look into compressing stuff more later. |
I tried looking at following the pattern in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes denoland/deno#3726 This PR provides support for referencing other lib files (like lib.dom.d.ts that are not used by default in Deno.
Fixes #3726
Still very much a work in progress, but I got so excited when I got it working.The following compiles without errors in Deno:It will lazily load thelib.dom.d.ts
which matches the version of TypeScript bundled with Deno fromgithub.com/microsoft/TypeScript
and cache it (as it would any other remote module).Instead of lazily loading them, we built them into the Deno binary.