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

chore: update webpack5 example to illustrate worker feedback. #456

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

w1nklr
Copy link
Contributor

@w1nklr w1nklr commented Jun 28, 2024

As a followup of #439, here comes an example of how to use workerpool.workerEmit() in a browser.

New example (left: before computations, right: with computations):
image

Enhanced readme to warn that the used workerpool package can not be up-to-date and add instructions to install latest package.

@w1nklr
Copy link
Contributor Author

w1nklr commented Jun 28, 2024

I wanted to add it to PR #455, but couldn't push it to Leonid's branch, and then it was too late :/

@josdejong
Copy link
Owner

Thanks @w1nklr, this looks good!

I think we should not expand this example further 😅, in general I try to give each example a focus and let it explain 1 thing else you don't see the trees from the forest.

One last idea: how about rewriting the example to for rexample React? All the manual management of document.createElement(...) etc inside a JSX file looks a bit odd 😁 (I think that was originally copied from a plain JS example).

@josdejong
Copy link
Owner

ping @w1nklr

@w1nklr
Copy link
Contributor Author

w1nklr commented Jul 19, 2024

Sorry, I'm quite busy these times...

Just checking I understood it correctly: you want to introduce a dependency in the example on react and rewrite this (and in the end all) example in react. Correct ?

@josdejong
Copy link
Owner

Yes indeed. But only if you feel like it.

We can leave it as it is for now, it was just an idea for further improvement.

@w1nklr
Copy link
Contributor Author

w1nklr commented Jul 19, 2024

I just updated this one example to react.
Let me know if this is what you want.

But build with node (22) fails at test level :(

@josdejong
Copy link
Owner

Thanks, I'll review the code soon.

The node 22 issue is not caused by this PR itself. It seems that there is something changed in the Github Actions or the latest Ubuntu version that it uses, it doesn't support using globals like rollup inside a project for some reason. I have something similar in an other project. No clue how to fix it though.

@josdejong
Copy link
Owner

The node 22 issue was introduced in v22.5.0 and fixed in v22.5.1 (see nodejs/node#53934)

@w1nklr
Copy link
Contributor Author

w1nklr commented Jul 24, 2024

Great.
The checks are green now !

@josdejong
Copy link
Owner

I've had a look at your latest updates, and this looks really neat! I love that you've split the example in three components, that keeps the example very clear and focused. Thanks!

<Fibonacci />
<FibonacciWithFeedback />
<TransferArray />

@josdejong josdejong merged commit cdb5861 into josdejong:master Jul 24, 2024
5 checks passed
@w1nklr
Copy link
Contributor Author

w1nklr commented Jul 24, 2024

You're welcome.
Thank you for the ping, this gave me the push I needed ;)

@w1nklr w1nklr deleted the feedback_example branch July 24, 2024 09:55
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.

2 participants