[image-manipulator] Introduce a new API for image manipulation on Web #30194
Conversation
|
The Pull Request introduced fingerprint changes against the base commit: 3bcb234 Fingerprint diff[
{
"op": "changed",
"source": {
"type": "dir",
"filePath": "../../packages/expo-modules-core",
"reasons": [
"expoAutolinkingIos",
"expoAutolinkingAndroid"
],
"hash": "8a34008159f55e3f7b9ec93bb08729d0f671fdfe"
}
}
]Generated by PR labeler 🤖 |
tsapeta
force-pushed
the
@tsapeta/web/image-manipulator-new-api
branch
from
87b8493 to
11cfcfc
Compare
tsapeta
force-pushed
the
@tsapeta/web/image-manipulator-new-api
branch
from
11cfcfc to
672109d
Compare
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
There was a problem hiding this comment.
This has been renamed to *.ts, but git shows it as deleted 🤷
There was a problem hiding this comment.
This file now contains equivalent tests but for the new API, the previous one are described as ImageManipulator (Legacy)
There was a problem hiding this comment.
Everything was just moved from ExpoImageManipulator.web.ts to here
| @@ -95,7 +95,7 @@ class SharedObject<TEventsMap extends Record<never, never>> | |||
| implements SharedObjectType | |||
| { | |||
| release(): void { | |||
| throw new Error('Method not implemented.'); | |||
| // no-op on Web, but subclasses can override it if needed. | |||
There was a problem hiding this comment.
We shouldn't throw from here by default as some methods (especially hooks like useReleasingSharedObject) can call this and catching the errors doesn't seem reasonable
Why
Following up on #30135
How
Implemented the new API for
expo-image-manipulatoron Web, added tests to test-suite and fixed a few small thingsTest Plan
For both legacy and the new API, NCL examples and tests work fine