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

Patch-package required to install the dependency #123

Open
LeoLeBras opened this issue Sep 12, 2024 · 3 comments
Open

Patch-package required to install the dependency #123

LeoLeBras opened this issue Sep 12, 2024 · 3 comments
Assignees
Labels
Platform: all Status: resolved Solved or Answered Type: enhancement New feature or request

Comments

@LeoLeBras
Copy link

LeoLeBras commented Sep 12, 2024

👋
I've always needed to make a patch package to avoid typescript errors and installation issue (I use a mono repo) ⬇️

diff --git a/node_modules/react-native-star-io10/react-native-star-io10.podspec b/node_modules/react-native-star-io10/react-native-star-io10.podspec
index 3c5142d..fd5a1c8 100755
--- a/node_modules/react-native-star-io10/react-native-star-io10.podspec
+++ b/node_modules/react-native-star-io10/react-native-star-io10.podspec
@@ -22,8 +22,8 @@ Pod::Spec.new do |s|
   s.dependency "React"
   s.pod_target_xcconfig = { 
     'EXCLUDED_ARCHS[sdk=iphoneos*]' => 'x86_64',
-    'EXCLUDED_SOURCE_FILE_NAMES[sdk=iphoneos*]' => '$(SRCROOT)/../../node_modules/react-native-star-io10/ios/libs/StarIO10.xcframework/ios-arm64_x86_64-simulator/*.*',
-    'FRAMEWORK_SEARCH_PATHS[sdk=iphoneos*]' => '$(SRCROOT)/libs/** $(SRCROOT)/../../node_modules/react-native-star-io10/ios/libs $(SRCROOT)/../../node_modules/react-native-star-io10/ios/libs/StarIO10.xcframework/ios-arm64',
+    'EXCLUDED_SOURCE_FILE_NAMES[sdk=iphoneos*]' => '$(PODS_TARGET_SRCROOT)/ios/libs/StarIO10.xcframework/ios-arm64_x86_64-simulator/*.*',
+    'FRAMEWORK_SEARCH_PATHS[sdk=iphoneos*]' => '$(PODS_TARGET_SRCROOT)/libs/** $(PODS_TARGET_SRCROOT)/ios/libs $(PODS_TARGET_SRCROOT)/ios/libs/StarIO10.xcframework/ios-arm64',
   }
   
   if ENV['USE_FRAMEWORKS']
diff --git a/node_modules/react-native-star-io10/src/StarIO10Logger.ts b/node_modules/react-native-star-io10/src/StarIO10Logger.ts
index a54f707..99ce8e7 100755
--- a/node_modules/react-native-star-io10/src/StarIO10Logger.ts
+++ b/node_modules/react-native-star-io10/src/StarIO10Logger.ts
@@ -48,5 +48,5 @@ export class StarIO10Logger extends NativeObject {
         return `dammy`;
     }
 
-    protected async _disposeNativeObjectImpl(nativeObject: string): Promise<void> {}
+    protected async _disposeNativeObjectImpl(): Promise<void> {}
 }
\ No newline at end of file
diff --git a/node_modules/react-native-star-io10/src/StarPrinter.ts b/node_modules/react-native-star-io10/src/StarPrinter.ts
index e301a16..0507ff4 100755
--- a/node_modules/react-native-star-io10/src/StarPrinter.ts
+++ b/node_modules/react-native-star-io10/src/StarPrinter.ts
@@ -262,8 +262,8 @@ export class StarPrinter extends NativeObject {
 
     async print(
         ...args:
-          | [command: string]
-          | [command: string, starSpoolJobSettings: StarSpoolJobSettings]
+          | [string]
+          | [string, StarSpoolJobSettings]
       ): Promise<void | number> {
         await this._initNativeObject();

Would you like a PR to correct this?

My react-native-info:

System:
    OS: macOS 14.1
    CPU: (12) arm64 Apple M2 Pro
    Memory: 47.80 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.4.0 - ~/.nvm/versions/node/v20.4.0/bin/node
    Yarn: 1.22.22 - /usr/local/bin/yarn
    npm: 9.7.2 - ~/.nvm/versions/node/v20.4.0/bin/npm
    Watchman: 2024.08.12.00 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.12.1 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 23.5, iOS 17.5, macOS 14.5, tvOS 17.5, visionOS 1.2, watchOS 10.5
    Android SDK: Not Found
  IDEs:
    Android Studio: Not Found
    Xcode: 15.4/15F31d - /usr/bin/xcodebuild
  Languages:
    Java: Not Found
    Python: Not Found
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.13.1
    react-native: 0.63.5
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

And my typescript version is 4.9.5.

@bandit-ibayashi bandit-ibayashi added Type: enhancement New feature or request Status: triage Start out by looking at issues Platform: all labels Oct 8, 2024
@can-miki
Copy link
Contributor

can-miki commented Oct 9, 2024

Thank you for sharing the information.
We are cautious about changing specifications because we must consider the impact on other customers already using our SDK.

I will look into a mono repo.
Please let us know if you have any reasons why we should, such as background, impact, effects, opinions, etc.

Thank you for always using our products and solutions.

@LeoLeBras
Copy link
Author

LeoLeBras commented Oct 9, 2024

In my patch package, there are two things:

  • patches for the invalid typescript code
  • support for monorepo

For the 2nd point, it's indeed important not to introduce a regression for users without monorepo. I've tested my patch package here https://github.com/star-micronics/react-native-star-io10/tree/master/example and everything works fine.

I don't know if I have an argument about whether or not to use monorepo. To be honest, in my tech team, we'd like to get away from this "monorepo" pattern/arch. But many react native packages have this monorepo support, it's become a common thing today.

@can-miki
Copy link
Contributor

can-miki commented Nov 28, 2024

Thank you for sharing the detailed information.

After internal discussions about the monorepo, we would like to improve the code of react-native-star-io10.podspec, as changing the podspec will not affect existing users.

Additionally, the information raised in the Issue included references to StarPrinter.ts and StarIO10Logger.ts, but we have determined that no action is necessary for these files because we created a sample of the monorepo structure and performed a simple check, which confirmed that if we make the adjustments to the podspec, we can use these files without any changes.

The timing for the implementation is likely to be several months away, so we would appreciate your patience.
If you have any other information or opinions, please feel free to reach out. Thank you for your cooperation.

@can-miki can-miki added Status: question Further information is requested and removed Status: triage Start out by looking at issues labels Nov 28, 2024
@can-miki can-miki added Status: resolved Solved or Answered and removed Status: question Further information is requested labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: all Status: resolved Solved or Answered Type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants