Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Fixing queue in a web worker #108

Merged
merged 3 commits into from
Aug 16, 2017
Merged

Fixing queue in a web worker #108

merged 3 commits into from
Aug 16, 2017

Conversation

rorticus
Copy link
Contributor

Type: bug

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

The queueTask method is broken right now if you are running from a web worker. This is because it's using postMessage (since it's available) but postMessage doesn't work like expected (it posts to the parent window in a web worker) and the tasks never get executed.

Resolves #107

@dylans dylans added this to the 2017.08 milestone Aug 14, 2017
@rorticus
Copy link
Contributor Author

ci failure is from the docs...

@@ -173,7 +173,7 @@ add('es6-weakmap', () => {

/* Miscellaneous features */
add('microtasks', () => has('es6-promise') || has('host-node') || has('dom-mutationobserver'), true);
add('postmessage', () => typeof global.postMessage === 'function', true);
add('postmessage', () => typeof global.window !== 'undefined' && typeof global.postMessage === 'function', true);
Copy link
Member

Choose a reason for hiding this comment

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

Not that it is a big deal, but it would be nice to add a comment here as to why we are doing the check this way, for future generations (think of the kids!), as it is non-obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a blurb.. you know, for the children.


const baseUrl = location.origin;
const dfd = this.async(10000);
const blob = new Blob([ `(function() {
Copy link
Member

Choose a reason for hiding this comment

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

Android 4.4.4 supports WebWorkers but doesn't support Blob creation this way. Again, not a big deal, but maybe add a simply blob check too?

Also, this might be a good pattern for doing these tests (thank you!) and might want to consider opening an issue on test-extras to add this as some sort of helper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up adding a simple global.Blob === undefined instead of a real Blob has check. The real Blob check is in core and has a few dependencies that we'd also have to copy over. Maybe that would be worth while at some point?

@rorticus rorticus merged commit cb7de79 into dojo:master Aug 16, 2017
@rorticus rorticus deleted the issue-107 branch August 16, 2017 11:49
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.

3 participants