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

Async actor remove cancelables #3371

Merged
merged 2 commits into from
Nov 15, 2023
Merged

Conversation

HarelM
Copy link
Member

@HarelM HarelM commented Nov 15, 2023

Removes more places that use cancelable in favor of abort controller.
Changes getVideo to return a promise instead of callback/cancelable.
Improve usage in async in tests and added a spy.

@codecov-commenter
Copy link

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (da95a03) 75.46% compared to head (1f0a97b) 75.46%.

Files Patch % Lines
src/source/video_source.ts 8.33% 11 Missing ⚠️
src/util/ajax.ts 40.00% 6 Missing ⚠️
src/ui/map.ts 71.42% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           async-actor    #3371   +/-   ##
============================================
  Coverage        75.46%   75.46%           
============================================
  Files              241      241           
  Lines            19214    19219    +5     
  Branches          4265     4264    -1     
============================================
+ Hits             14500    14504    +4     
- Misses            4714     4715    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@neodescis neodescis left a comment

Choose a reason for hiding this comment

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

LGTM!

@HarelM HarelM merged commit 9feae22 into async-actor Nov 15, 2023
12 checks passed
@HarelM HarelM deleted the async-actor-remove-cancelable branch November 15, 2023 21:50
HarelM added a commit that referenced this pull request Nov 23, 2023
* Initial concept

* More improvements

* Fix lint

* Main commit to move most of the actor code to use promises and register. There are still things to do.

* Fix actor tests

* Remove more any when it comes to worker self typings.

* Improve more types, add more tests

* Improve more types, add more tests

* Fix lint

* Fix more tests

* Fix tests

* Fix some sytle tests

* Add comment for todo.

* Fix test related to message parameters change

* Fix remaining tests

* Added todo to accomodate for code review.

* Fix build test

* Fix geojson types

* Rename some events, remove clutter a bit

* Fix issue with multiple maps found in integration tests

* Fix tests and improve types and docs a bit more

* Final fixes to make the tests pass

* Fix build test

* a couple of tweaks (#3267)

* Add changelog item

* More improvement to async `Actor` and replace more callbacks with promises  (#3269)

* Improve vector tile source

* Fix unit tests

* Fix render tests for the case of 404

* Fix lint

* Remove unused method

* More improvements to remove callbacks and move to promises

* Fixed some tests, more fixes are needed.

* Fix remaining tests.

* Fix lint

* Fix tile parser code to pass render tests

* Fix lint

* Fix lint

* More fixes and tests in the infrasturcure when it comes to cancelling

* forgot to inclue this...

* Fix lint and build

* Convert parse to be a proper async method.

* Add more test coverate to actor, allow more testing in mock web worker.

* Replace getJson call with async code and remove more callbacks (#3273)

* Add abort between actors of main thread and worker thread with tests

* Fix lint

* Fix lint

* Fix node from lts to nvmrc

* Improve the code of vector tile and geojson workers

* Fix lint

* Add debug when tests are failing

* Do not throw in case of empty geojson tile

* Fix lint

* Move geojson worker source to be more async like.

* Change getJSON to be async extenally

* Update src/ui/map.test.ts

Co-authored-by: Bart Louwers <bart.louwers@gmail.com>

---------

Co-authored-by: Bart Louwers <bart.louwers@gmail.com>

* Move image queue to use promises, move getArrayBuffer to use promises (#3280)

* Migrate getArrayBuffer to be promise based

* Remove dead code in ajax.ts file

* Code review changes (#3278)

Co-authored-by: neodescis <nsteinbaugh@icr-team.com>

* Make image request return promises.

* Move process queue to the right location

* Increase build size

* Last fixes

* fix lint warning

* Fix lint

* update docs

Co-authored-by: neodescis <nsteinbaugh@gmail.com>

* Update docs

Co-authored-by: neodescis <nsteinbaugh@gmail.com>

* Fix docs

* Code review comments.

* Update src/util/image_request.ts

* Update src/util/util.ts

---------

Co-authored-by: neodescis <nsteinbaugh@gmail.com>
Co-authored-by: neodescis <nsteinbaugh@icr-team.com>

* Remove TODOs, tidy up last things (#3301)

* Remove some TODOs

* Remove some todos, make ajax API fully promise based

* Move tileJson to be async

* fix lint

* Remove another TODO

* Move makexmlhttprequest to return a promise

* Make load_sprite async

* Added relevant tests

* Final fixes

* Update src/source/vector_tile_source.test.ts

Co-authored-by: neodescis <nsteinbaugh@gmail.com>

---------

Co-authored-by: neodescis <nsteinbaugh@gmail.com>

* Async actor docs additions (#3305)

* Fix missing classes in the docs

* Fixed the relevant docs in the places where API calls were changed.

* Fix lint

* Remove some "then" from tests

* remove todos, remove "then"s, simplify the ajax code a bit

* Remove as any from where it's not needed

* Fix code review expect stuff...

* explicit promises instead of array.

* Improve ajax further

* Improve docs for RTL plugin status and reduce code in geojson worker test

* Last simplification to actor, simplify a style test.

* Add missing docs comments

* Fix lint...

* Fix Actor XSS, introduce subscribe (#3329)

* Fix XSS, intorduce subscribe

* Update changelog

* Fix developer diagram with updated flow

* Async actor no log warnings and errors in unit tests (#3368)

* Upgrade jest, fix tests, remove console log messages.

* Fix incorrect additions, lint

* Async actor remove cancelables (#3371)

* Remove more places that use cancelable

* Remove more places with canceable

* Async actor callback and promise (#3374)

* Remove more places that use cancelable

* Remove more places with canceable

* callback and promise

* Improve sources code when it is related to callback

* Use callback failback when needed

* Update build size test

* Removed callbacks from various places, adding docs, simplifying tests

* Improve type

* Update src/util/actor.test.ts

Co-authored-by: neodescis <nsteinbaugh@gmail.com>

* Make load async for all sources

---------

Co-authored-by: neodescis <nsteinbaugh@gmail.com>

* Update changelog with most of the changes

---------

Co-authored-by: Michael Barry <msbarry@users.noreply.github.com>
Co-authored-by: Bart Louwers <bart.louwers@gmail.com>
Co-authored-by: neodescis <nsteinbaugh@gmail.com>
Co-authored-by: neodescis <nsteinbaugh@icr-team.com>
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

3 participants