Comments (9)
Thanks for the example that will be an easy fix 👍
I think I will make it the default, with an option to turn it off. I want most users to be able to have zero-config, and it seems more common to want to strip the patches than to preserve them.
Yes, each function is it's own package to keep them small. I'm using GCP functions directly rather than firebase, and using terraform to deploy the resources. I use turborepo to generate function template folders and isolate-package is part of that now :)
Interesting. Terraform is something that was on my to-study list and I didn't know turborepo could generate template folders. I think I still would struggle a bit navigating that many packages, but it's good to know about the approach.
Thanks for the kind words. I will try to implement this soon.
from isolate-package.
@erik-lissen Thanks for reporting. I have no experience with patched dependencies. Could you share the lockfile maybe, so I get a better idea where they need to be stripped?
Do I understand correctly that there is no use-case for having patched dependencies as part of an isolated package lockfile, and therefore they can always be removed?
from isolate-package.
I read a bit about patching dependencies and why it is done.
As I understand it, it would be essential to keep the patch part of the isolated package, otherwise the fix will be undone in the deployed code. So I suspect that a solution could be:
- Make sure the patch file ends up in the output bundle
- Adapt the lockfile so that it points to the correct patch filepath.
What do you think?
If this feature is important to you, maybe try to submit a PR, because I do not think I will be working on this very shortly. I have too much on my plate already and this seems like a very rare use-case.
Alternatively, you could try to force output to generate an NPM lockfile with the forceNpm
configuration option. Maybe that solves something, but I doubt it because the patch would probably still need to be copied to the isolate output.
from isolate-package.
(in my case, this patch is from my frontend code which has no business breaking my backend)
Forgive me, now I understand what you meant with this line.
So even though patches could be essential for deployed code, we need the option to strip them from the lockfile because the patches might not apply to the code that is being isolated.
In that case, I think the fix could be easy, and I'm willing to look at it if you show me your lockfile.
Also, my suggestion for forceNpm
then seems like a valid thing to try in the meantime. The lockfile adaption logic is different between pnpm and npm. In case of pnpm the lockfile is adapted using my own logic, in the case of npm the lockfile is regenerated by a library that is part of npm itself.
Note that your codebase can still use pnpm, it's just that the option generates an npm compatible isolate.
from isolate-package.
So even though patches could be essential for deployed code, we need the option to strip them from the lockfile because the patches might not apply to the code that is being isolated.
Yes, it's trying to patch something that is not in my isolated package.
In that case, I think the fix could be easy, and I'm willing to look at it if you show me your lockfile.
That would be great - it's a minor inconvenience not to have it, not a huge deal. Right now, we don't have any patches for the backend code, so I just run a node script to strip out the patched deps, job done. But if I had some patches on my functions it would be a lot more problematic.
Also, my suggestion for
forceNpm
then seems like a valid thing to try in the meantime. The lockfile adaption logic is different between pnpm and npm. In case of pnpm the lockfile is adapted using my own logic, in the case of npm the lockfile is regenerated by a library that is part of npm itself.
I tried forceNpm, but it has another problem - I'm using turborepo to isolate many packages all at once - because of how the node_modules folder is moved in-out of its place with forceNpm, it can only be done one-by-one which will take too long (I have 35 functions at the moment) - as the node_modules folder gets deleted while something else is trying to use it.
from isolate-package.
I tried forceNpm, but it has another problem - I'm using turborepo to isolate many packages all at once - because of how the node_modules folder is moved in-out of its place with forceNpm, it can only be done one-by-one which will take too long (I have 35 functions at the moment) - as the node_modules folder gets deleted while something else is trying to use it.
Ah right, that's an unfortunate side effect. Do you create a monorepo package for each cloud function you deploy? Is that to minimize cold-start times?
I'm just curious, because I have 30+ functions in my project as well, but they are only spread out over a few packages, and I never really considered breaking it up into one per package.
I think that patching dependencies, because it seems often related to using bleeding-edge libs, is more likely to be done on the frontend than backend. So I assume stripping it from the lockfile is sufficient for 99.9% of users. I'm not aware of any client-side deployments requiring an isolated package.
from isolate-package.
Ah right, that's an unfortunate side effect. Do you create a monorepo package for each cloud function you deploy? Is that to minimize cold-start times?
Yes, each function is it's own package to keep them small. I'm using GCP functions directly rather than firebase, and using terraform to deploy the resources. I use turborepo to generate function template folders and isolate-package is part of that now :)
Great package btw, I solved this myself previously with my own convoluted cli tool, but now that I moved to using turborepo your package is a huge help!
I think that patching dependencies, because it seems often related to using bleeding-edge libs, is more likely to be done on the frontend than backend. So I assume stripping it from the lockfile is sufficient for 99.9% of users. I'm not aware of any client-side deployments requiring an isolated package.
Yes, I think I agree with that statement. Just stripping all of it is probably sufficient. Otherwise, you'd have to check for any patches, go through the dependencies and check if any of those are included in the current package and strip the ones out that are not. I haven't looked at your code too much so I don't know if that's hard or not
Right now I'm just doing this for all my functions:
"scripts": {
"isolate": "isolate && pnpm run removePatched",
"removePatched": "node -e \"const fs = require('fs'), yaml = require('js-yaml'); let yamlData = yaml.load(fs.readFileSync('./isolated/pnpm-lock.yaml', 'utf8')); delete yamlData.patchedDependencies; fs.writeFileSync('./isolated/pnpm-lock.yaml', yaml.dump(yamlData), 'utf8');\""
},
I would suggest just adding an option like stripPatchedDependencies
that does that ⬆️
from isolate-package.
@erik-lissen I have released 1.10.0-1 under @next so could you give it a try and let me know if it works for you? I didn't test it myself, but I think this should work...
from isolate-package.
Should be fixed by 1.10.0, although I frankly didn't test it :)
from isolate-package.
Related Issues (20)
- Deploy multiple functions codebases from root HOT 5
- Isolation processed triggered twice
- Strip forceNpm option for sake of simplicity
- Reintroduce excludeLockfile
- Add support for pnpm v9 HOT 4
- trouble shooting some problems HOT 3
- Consistently use pnpm pack if suitable
- Adopt packageManager field from root manifest
- Add flag to omit packageManager field
- Define config using a typed function
- Option to prevent move of node_modules for NPM HOT 3
- Will fail if pnpm-workspace.yaml is empty HOT 2
- Handle non-monorepo setups
- Isolated pnpm lockfile is missing overrides HOT 4
- Having some problems isolating a package HOT 5
- Improve support for Windows
- Isolate fails due to missing pnpm HOT 5
- Isolate fails due to missing pnpm on Windows
- Detect for all packages if they have a files field in manifest
- PNPM prune fails to read 'specifiers' HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from isolate-package.