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

Adding option to patch "all" files, including package.json #2

Open
akuul opened this issue Sep 13, 2024 · 4 comments
Open

Adding option to patch "all" files, including package.json #2

akuul opened this issue Sep 13, 2024 · 4 comments
Labels
enhancement New feature or request Fixed

Comments

@akuul
Copy link

akuul commented Sep 13, 2024

It seem custompatch is ignoring package.json files at the moment. Could we get an options something like --all to be able to patch packages package.json files, similar way --exclude 'all' works for patch-package ?

@tmcdos
Copy link
Owner

tmcdos commented Sep 13, 2024

I will look into this problem, give me some time.

@tmcdos tmcdos added the enhancement New feature or request label Sep 13, 2024
tmcdos added a commit that referenced this issue Sep 15, 2024
@tmcdos
Copy link
Owner

tmcdos commented Sep 15, 2024

Can you try the new version? I have added an -a or --all option to include package.json in the patch files. There have been discussions in the past about patch-package and applying patches to package.json and as far as I know - NPM does not provide a lifecycle hook for any event after downloading a package and before resolving its dependencies. So even if you try to patch a dependency in some package.json - it probably won't be detected by npm install. You may try the overrides section in your root package.json

@tmcdos tmcdos added the Fixed label Sep 15, 2024
@akuul
Copy link
Author

akuul commented Sep 17, 2024

Thanks you for the enchantment.

-a options seems to indeed create necessary patch with package.json included. However, there are few issues I currently have.

Pathcing package.json adds additional artefacts and I'm not sure if that's intended so maybe you could clarify:

Index: /reactotron-core-client/package.json
===================================================================
--- /reactotron-core-client/package.json
+++ /reactotron-core-client/package.json
@@ -18,9 +18,9 @@
<...>
-    "react-native": "./src/index.ts",
+    "react-native": "./dist/index.ts",
<...>
-
-,"_resolved": ""
-,"_integrity": ""
-,"_from": "https://registry.npmjs.org/reactotron-core-client/-/reactotron-core-client-2.9.4.tgz"
 }
\ No newline at end of file

The changes were done on "./dist/index.ts".

Not sure if related, but I'm also getting WARNING: Chunk failed - either already applied or for different version specifically for package.json only when applying patch and I can confirm that the changes are actually not applied. Patch for createSocket is being applied properly.

I am using yarn 4.4.1 ( Berry) if it helps in any way.

Full patch:

Index: /reactotron-core-client/package.json
===================================================================
--- /reactotron-core-client/package.json
+++ /reactotron-core-client/package.json
@@ -18,9 +18,9 @@
   "types": "dist/types/src/index.d.ts",
   "react-native": "src/index.ts",
   "exports": {
     "import": "./dist/index.esm.js",
-    "react-native": "./src/index.ts",
+    "react-native": "./dist/index.ts",
     "types": "./dist/types/src/index.d.ts",
     "default": "./dist/index.js"
   },
   "scripts": {
@@ -102,9 +102,5 @@
   },
   "dependencies": {
     "reactotron-core-contract": "0.2.4"
   }
-
-,"_resolved": ""
-,"_integrity": ""
-,"_from": "https://registry.npmjs.org/reactotron-core-client/-/reactotron-core-client-2.9.4.tgz"
 }
\ No newline at end of file
Index: /reactotron-core-client/src/client-options.ts
===================================================================
--- /reactotron-core-client/src/client-options.ts
+++ /reactotron-core-client/src/client-options.ts
@@ -12,9 +12,9 @@
    *
    * This is over-engineered because we need the ability to create different
    * types of websockets for React Native, React, and NodeJS.  :|
    */
-  createSocket?: ((path: string) => BrowserWebSocket) | ((path: string) => NodeWebSocket)
+  createSocket?: ((path?: string) => BrowserWebSocket) | ((path: string) => NodeWebSocket)
 
   /**
    * The hostname or ip address of the server.  Default: localhost.
    */

@tmcdos
Copy link
Owner

tmcdos commented Sep 18, 2024

Indeed, the issue that you're observing was the primary cause that I originally did not include package.json in the patches - the code on line 280 has been commented out because I could not find a reliable solution. You can try uncommenting it and test if it will work with your particular case.
I believe that older versions of npm (e.g. below v8 or maybe even v9) change the contents of non-root package.json files and thus breaking the patching process (the diff no longer matches the original untouched package.json).
You can also try with a more recent npm (e.g. v10) instead of yarn - just to see if the issue disappears.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Fixed
Projects
None yet
Development

No branches or pull requests

2 participants