Skip to content

Enable passing extra args to load function in classes derived from BaseModule#381

Merged
pweglik merged 1 commit intomainfrom
@pw/fix-load-extra-args
Jun 12, 2025
Merged

Enable passing extra args to load function in classes derived from BaseModule#381
pweglik merged 1 commit intomainfrom
@pw/fix-load-extra-args

Conversation

@pweglik
Copy link
Copy Markdown
Contributor

@pweglik pweglik commented Jun 9, 2025

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (improves or adds clarity to existing documentation)

Tested on

  • iOS
  • Android

Testing instructions

Tested in CV object detection - it works

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional notes

@pweglik pweglik requested review from chmjkb and mkopcins June 9, 2025 12:14
@pweglik pweglik self-assigned this Jun 9, 2025
...sources
);
await this.nativeModule.loadModule(...paths);
await this.nativeModule.loadModule(...paths, ...loadArgs);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I dont think nativeModule accepts loadArgs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, in some cases it can? What if we want to pass an extra argument to the native side load function? I see no downside. When you call this in derived class:

await super.load([modelSource, tokenizerSource], someArg);

you can expect that everything goes to the native side as it should

@pweglik pweglik requested a review from mkopcins June 10, 2025 09:36
@pweglik pweglik force-pushed the @pw/fix-load-extra-args branch from 52a436a to aeb603f Compare June 10, 2025 11:37
@pweglik pweglik force-pushed the @pw/fix-load-extra-args branch from aeb603f to 2a541e8 Compare June 12, 2025 10:28
@pweglik pweglik merged commit df4eae4 into main Jun 12, 2025
2 checks passed
@pweglik pweglik deleted the @pw/fix-load-extra-args branch June 12, 2025 10:58
mkopcins pushed a commit that referenced this pull request Oct 15, 2025
…seModule (#381)

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Documentation update (improves or adds clarity to existing
documentation)

### Tested on

- [x] iOS
- [ ] Android

### Testing instructions

Tested in CV object detection - it works

### Checklist

- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [x] My changes generate no new warnings

### Additional notes

<!-- Include any additional information, assumptions, or context that
reviewers might need to understand this PR. -->
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