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

Add worker decorator #1147

Closed
wants to merge 5 commits into from
Closed

Add worker decorator #1147

wants to merge 5 commits into from

Conversation

splincode
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

@splincode splincode changed the title Feat/worker Add worker decorator Dec 9, 2021
@@ -21,7 +21,7 @@ export function Debounce(timeout: number = DEFAULT_TIMEOUT): MethodDecorator {
timeoutRef = window.setTimeout((): void => {
const result: Any = originalMethod.apply(this, args);

if (isDevMode() && isNotNil(result)) {
if (isDevMode() && Boolean(isNotNil(result))) {
Copy link
Member

Choose a reason for hiding this comment

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

accidentally?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch

@@ -0,0 +1,15 @@
import { Any, Descriptor } from '@angular-ru/cdk/typings';

import { WebWorkerThreadService as Thread } from './worker-thread.service';
Copy link
Member

Choose a reason for hiding this comment

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

maybe it will be better to import WorkerThread?

@Markiewic
Copy link
Member

@splincode

I got such an idea about workers with webpack loader. We have three problems with current solution:

  1. We can pass into WorkerService inline function which is using lexical environment. So since the function will be converted to string, it won't work.
  2. We can't import into worker function any symbols, cause they will be imported to the general thread.
  3. We can't point to the vars from the general thread. We can only serialize and post them as string.

So i suggest to make a webworker webpack loader. It will force user to separate the side function at least outside the lexical environment (namely, to the separate file) — 1.

import('worker-loader!./some-fn.worker.ts').then(({ someFn }) => someFn(someData))

As we move the code to separate file, webpack can correctly import any symbols to it — 2. The third problem can be solved with SharedArrayBuffer, but I'm not sure.

Wyt? Isn't it an overhead?

@splincode
Copy link
Member Author

image

import('worker-loader!./some-fn.worker.ts').then(({ someFn }) => someFn(someData))

not needed, it's work by default in angular cli

@splincode
Copy link
Member Author

splincode commented Dec 10, 2021

our webworker service - it's redundant for production, but only just for fun utils. We need migrate to native webpack loader from angular

@angular-ru-bot angular-ru-bot force-pushed the feat/worker branch 7 times, most recently from ed9da54 to 8c1e743 Compare December 17, 2021 14:30
@angular-ru-bot angular-ru-bot force-pushed the feat/worker branch 7 times, most recently from 1252b9b to 720da90 Compare December 20, 2021 21:11
@Markiewic
Copy link
Member

Do you comment anything here? Cause i'm receiving notifications periodically, but see not updates.

@splincode
Copy link
Member Author

@Schirbak hello, postponed, I will answer your suggestions asap

@splincode splincode closed this Jul 6, 2024
@splincode splincode deleted the feat/worker branch July 6, 2024 20:36
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.

None yet

2 participants