Skip to content

feat(@ngtools/webpack): Automatically bundle module workers #12575

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

Closed

Conversation

developit
Copy link
Contributor

@developit developit commented Oct 12, 2018

This pull request adds support for automatic bundling of module Workers using worker-plugin.

In essence, it teaches Angular CLI to compile this standard browser syntax:

const worker = new Worker('./some/path', { type: 'module' });

... into this optimized output, bundling the worker code as if it were a split point:

const worker = new Worker('0.worker.12345.js');

/cc @robwormald

@developit developit force-pushed the feature/automatically-bundle-workers branch from bd0d14f to 1960433 Compare October 12, 2018 20:39
@developit developit changed the title feature(workers): Automatically bundle module workers feat(@ngtools/webpack): Automatically bundle module workers Oct 12, 2018
Supports new Worker(...,{type:module}) using github.com/googlechromelabs/worker-plugin
@developit developit force-pushed the feature/automatically-bundle-workers branch from 1960433 to 0a337ef Compare October 12, 2018 20:57
@filipesilva filipesilva added the needs: discussion On the agenda for team meeting to determine next steps label Oct 15, 2018
@IgorMinar
Copy link
Contributor

I've looked into this briefly, and in order to get this in, we'll need at least the following:

  • support for IE11 (no type: module)
  • ensure that I can use typescript in the worker modules and that these modules are built as a separate compilation unit with separate tsconfig-worker.json
  • an e2e tests in the CLI repo (with worker code written in typescript, one version with type: "module", and one without)
  • brief documentation on angular.io (location tbd)
  • bike-shed autoBundleWorkerModules name, it doesn't seem to describe the feature quite accurately. how about buildWorkerEntryPoints?
  • confirm that we want enable this by default (I think so, as long as it's possible to turn it off)

@developit
Copy link
Contributor Author

Hi @IgorMinar - just clarifying:

  • The output does not use { type:module }. It is compiled to a "classic" Worker. The module type is simply used as a compilation hint and to align the semantics of modules being availble within the worker context due to Webpack being run there.
  • I've used this in TypeScript projects and it works - configuration is inherited from the parent compilation. I'm not sure about typescript-worker.json support.
  • I'm ambivalent on the name, the current one came from @robwormald

@alexeagle
Copy link
Contributor

Design doc (requires explicit sharing with individuals: https://docs.google.com/document/d/10Ok6pn6L8VoJUys6shqSYKDSRxmu1fM2hAix7PFg4oQ/edit )

@alexeagle alexeagle requested a review from IgorMinar October 18, 2018 17:23
@alexeagle alexeagle removed the needs: discussion On the agenda for team meeting to determine next steps label Oct 18, 2018
@developit
Copy link
Contributor Author

Thanks @alexeagle. Please let me know if there's anything that needs clarification or that I can help out with. I've spoken to a fair number of folks about this and there's a lot of excitement around getting it merged.

@IgorMinar
Copy link
Contributor

additional input into the design discussion: https://www.npmjs.com/package/worker-loader

that loader seems too magical and webpack specific, I prefer @developit's approach more.

@ngbot
Copy link

ngbot bot commented Oct 31, 2018

Hi @developit! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@developit
Copy link
Contributor Author

Merge conflicts resolved.

@alexeagle
Copy link
Contributor

@developit I don't want this to get lost - but Igor has a checklist above of things we need to be able to merge this. Do you have time to do some or all of those things? Otherwise we need to find someone else to get it over the finish line.

@alexeagle alexeagle added the needs: discussion On the agenda for team meeting to determine next steps label Nov 20, 2018
@LanceEa
Copy link

LanceEa commented Dec 17, 2018

@developit @alexeagle - this would be a cool feature and would love to see it land. Any plan to move forward with this or any help needed? I can commit some time to help out wherever needed to get it over the finish line.

@mgechev
Copy link
Member

mgechev commented Dec 17, 2018

@LanceEa, we're working on landing this.

@daporro
Copy link

daporro commented Mar 11, 2019

why was this closed?

@alexzuza
Copy link

@daporro I think it was closed in favor of #13700

@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 12, 2019
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.

9 participants