Skip to content

Web Worker support #13700

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

Merged
merged 6 commits into from
Apr 2, 2019
Merged

Web Worker support #13700

merged 6 commits into from
Apr 2, 2019

Conversation

filipesilva
Copy link
Contributor

@filipesilva filipesilva commented Feb 18, 2019

Followup to #12575

Todo:

Close #5885

@Serginho
Copy link

Serginho commented Feb 19, 2019

@filipesilva I'm following this feature closely and I can see worker plugin won't support lazy loading. it's only a suggestion.

Why don't you bundle the workers using webpack multiple targets?
export.module = [{target: 'web', ....}, {target: 'webworker' ...}]

Or maybe you are not doing this thinking in platform-webworker?

@filipesilva
Copy link
Contributor Author

@Serginho at the moment I'm just following the base functionality of https://github.com/googlechromelabs/worker-plugin. It only supports { type:'module' }. The functionality as is right now isn't meant to support @angular/platform-webworker, but rather just to bundle web workers.

@filipesilva filipesilva force-pushed the pr12575-followup branch 2 times, most recently from 651a873 to c4d7750 Compare February 21, 2019 14:52
@filipesilva filipesilva added the needs: discussion On the agenda for team meeting to determine next steps label Feb 21, 2019
@filipesilva filipesilva force-pushed the pr12575-followup branch 2 times, most recently from 0717c0a to 7471ca1 Compare February 25, 2019 19:22
@filipesilva filipesilva force-pushed the pr12575-followup branch 2 times, most recently from 7bcfd12 to b35f1ad Compare February 28, 2019 17:43
@mgechev mgechev removed the needs: discussion On the agenda for team meeting to determine next steps label Feb 28, 2019
@filipesilva filipesilva force-pushed the pr12575-followup branch 2 times, most recently from c7041a1 to d361f51 Compare February 28, 2019 18:31
@filipesilva filipesilva force-pushed the pr12575-followup branch 2 times, most recently from 638fc11 to 8c14243 Compare March 6, 2019 17:37
// Add project tsconfig.json and tsconfig.worker.json.
// The project level tsconfig.json with webworker lib is for editor support since
// the dom and webworker libs are mutually exclusive.
// Note: this schematic does not change other tsconfigs to use the project-level tsconfig.
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 schematic makes a two new tsconfig files:

  • path/to/project/tsconfig.worker.json for the worker TS compilation itself
  • path/to/project/tsconfig.json for the editor to support both DOM and WebWorker typings inside the project

As is I did not make the remaining tsconfigs in path/to/project/ extend path/to/project/tsconfig.json because it's hard to ensure they are the default tsconfigs we generate on a new project.

But I believe we should eventually transition into having a path/to/project/tsconfig.json anyway in all projects because of other editor support issues like #8138 (comment).

// 'src/worker/dep.ts'.
// But increasing this delay to 5s lead to no failed test in over 40 runs locally.
// In CI I still saw a rebuild fail with 5s, so I increased it to 10s.
// I think there might be a race condition related to child compilers somewhere in webpack.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still saw a failure with the 10s debounce time in https://circleci.com/gh/angular/angular-cli/43023#tests/containers/1. Need to debug further.

Copy link
Contributor

Choose a reason for hiding this comment

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

@filipesilva Is it possible the child compiler needs additional plugins? If it does and their configuration is the same, adding a string match pattern to the plugins[] option in worker-plugin would copy them over.

I'm not sure where to look in this repo to peek at how file watching is being done, any tips appreciated!

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'm looking at this currently. I'm not 100% sure of what the problem is but I've managed to reproduce it locally. I think it's related to the shared bits of our TS compilation pipeline.

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 think I figured it out, let's continue the conversation in the other comment (#13700 (comment)).

@owenhaynes
Copy link

owenhaynes commented Mar 18, 2019

Will this also enable shared workers as well?

@filipesilva
Copy link
Contributor Author

@owenhaynes I'm not sure what you mean, can you give an example please?

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

This is really good!

Couple of small things above.

@filipesilva filipesilva force-pushed the pr12575-followup branch 4 times, most recently from 1e3e52e to 44dbe6e Compare April 1, 2019 09:29
@filipesilva filipesilva changed the title bundle module workers followup Web Worker support Apr 1, 2019
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM

jasonaden pushed a commit to angular/angular that referenced this pull request Apr 1, 2019
@mgechev mgechev merged commit 6e0a040 into angular:master Apr 2, 2019
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Apr 10, 2019
…lder

Followup to angular#13700, the karma builder did not get the same support but it should.
alexeagle pushed a commit that referenced this pull request Apr 10, 2019
…lder

Followup to #13700, the karma builder did not get the same support but it should.
@filipesilva filipesilva deleted the pr12575-followup branch April 15, 2019 15:03
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to work with web workers (use a web worker IN cli project)
9 participants