Skip to content

refactor: remove BaseModule.ts and rename NonStaticBaseModule.ts to replace it.#512

Merged
mlodyjesienin merged 1 commit intomainfrom
@mlodyjesienin/refactor-basemodel
Aug 29, 2025
Merged

refactor: remove BaseModule.ts and rename NonStaticBaseModule.ts to replace it.#512
mlodyjesienin merged 1 commit intomainfrom
@mlodyjesienin/refactor-basemodel

Conversation

@mlodyjesienin
Copy link
Copy Markdown
Contributor

Description

After the (almost full) port to C++, the static BaseModule in typescript is no longer needed.
Additionally NonStaticBaseModule can take its place and use simpler, less confusing name.
This way, in most cases the class that inherits after BaseModule corresponds nicely with C++ class that inherits after BaseModel C++ class.

The same reasoning could be made, and therefore was applied to hooks: useModule and useNonStaticModule

Introduces a breaking change?

  • Yes
  • No

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Other (chores, tests, code style improvements etc.)

Tested on

  • iOS
  • Android

Testing instructions

Screenshots

Related issues

Closes #500

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

@mlodyjesienin mlodyjesienin requested a review from jakmro August 20, 2025 08:41
@mlodyjesienin mlodyjesienin self-assigned this Aug 20, 2025
@mkopcins
Copy link
Copy Markdown
Collaborator

lets wait with this until we implement the fix to llms and migrate them to JSI

@mlodyjesienin mlodyjesienin force-pushed the @mlodyjesienin/refactor-basemodel branch from 66c4abc to 1b70898 Compare August 28, 2025 11:49
@mlodyjesienin mlodyjesienin requested a review from mkopcins August 28, 2025 11:50
@mlodyjesienin mlodyjesienin merged commit e9d0a9e into main Aug 29, 2025
2 checks passed
@mlodyjesienin mlodyjesienin deleted the @mlodyjesienin/refactor-basemodel branch August 29, 2025 08:25
mkopcins pushed a commit that referenced this pull request Sep 2, 2025
…eplace it. (#512)

After the port to C++, the static `BaseModule` in
typescript is no longer needed.
Additionally `NonStaticBaseModule` can take its place and use simpler,
less confusing name.
This way, in most cases the class that inherits after `BaseModule`
corresponds nicely with C++ class that inherits after `BaseModel` C++
class.

The same reasoning could be made, and therefore was applied to hooks:
`useModule` and `useNonStaticModule`

- [ ] Yes
- [x] No

- [ ] Bug fix (change which fixes an issue)
- [ ] New feature (change which adds functionality)
- [ ] Documentation update (improves or adds clarity to existing
documentation)
- [x] Other (chores, tests, code style improvements etc.)

- [x] iOS
- [ ] Android

<!-- Provide step-by-step instructions on how to test your changes.
Include setup details if necessary. -->

<!-- Add screenshots here, if applicable -->

Closes #500

- [x] 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
- [x] My changes generate no new warnings

<!-- Include any additional information, assumptions, or context that
reviewers might need to understand this PR. -->
KnextKoder pushed a commit to Synkhiv/react-native-executorch that referenced this pull request Nov 7, 2025
…eplace it. (software-mansion#512)

## Description

After the port to C++, the static `BaseModule` in
typescript is no longer needed.
Additionally `NonStaticBaseModule` can take its place and use simpler,
less confusing name.
This way, in most cases the class that inherits after `BaseModule`
corresponds nicely with C++ class that inherits after `BaseModel` C++
class.

The same reasoning could be made, and therefore was applied to hooks:
`useModule` and `useNonStaticModule`

### Introduces a breaking change?

- [ ] Yes
- [x] No

### Type of change

- [ ] Bug fix (change which fixes an issue)
- [ ] New feature (change which adds functionality)
- [ ] Documentation update (improves or adds clarity to existing
documentation)
- [x] Other (chores, tests, code style improvements etc.)

### Tested on

- [x] iOS
- [ ] Android

### Testing instructions

<!-- Provide step-by-step instructions on how to test your changes.
Include setup details if necessary. -->

### Screenshots

<!-- Add screenshots here, if applicable -->

### Related issues

Closes software-mansion#500 

### Checklist

- [x] 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
- [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.

Refactor BaseModel and BaseNonStaticModule in Typescript

2 participants