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

Implement Blob url support for worker #2729

Merged
merged 2 commits into from
Aug 6, 2019

Conversation

kevinkassimo
Copy link
Contributor

@kevinkassimo kevinkassimo commented Aug 5, 2019

Related to #2726

Support worker creation using Blob URL.

Notice that it does not support static import statements (It seems to me that Chrome also does not support it with both blob url and data url?)

(Data URL version is left to implement later since there are more complications involved)

@@ -63,6 +66,20 @@ function parse(url: string): URLParts | undefined {
return undefined;
}

// Based on https://github.com/kelektiv/node-uuid
// TODO(kevinkassimo): Use deno_std version once possible.
Copy link
Member

Choose a reason for hiding this comment

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

👍
I think I now have an ideas now on how to use deno_std well (#2711)

@@ -0,0 +1 @@
code from Blob
Copy link
Member

Choose a reason for hiding this comment

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

cool.

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - no comments

@ry ry merged commit ccee2f0 into denoland:master Aug 6, 2019
@piscisaureus piscisaureus mentioned this pull request Aug 9, 2019
@qwerasd205
Copy link
Contributor

I figured I'd go ahead and reference #2797 here since this is where the feature with the bug in it was created. (summary of #2797: Workers created from blobs cause panic when error is thrown). If I shouldn't bring up issues on merged PRs please let me know for future reference.

@EliteScientist
Copy link

Was this reverted in 1.4.1? I'm having issues with my project because of this:

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.

4 participants