-
Notifications
You must be signed in to change notification settings - Fork 38
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
Reduce code duplication #57
Conversation
801c945
to
6adddf1
Compare
I could also help with this. To orchestrate the packages we could set up a simple NPM workspace in the
When everything is setup correctly you can then import { example } from 'lib';
// ... |
Another upside to the workspace approach is you can share |
src/node/addon/package.json
Outdated
@@ -19,6 +19,7 @@ | |||
"debrid-link-api": "^1.0.1", | |||
"express-rate-limit": "^6.7.0", | |||
"ip": "^1.1.8", | |||
"knightcrawler-utility": "file:../utility", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefixing with the project name isn't really required as this utility package is never exposed (i.e. installable) to the outside world.
src/node/utility/package.json
Outdated
@@ -0,0 +1,9 @@ | |||
{ | |||
"name": "knightcrawler-utility", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively you can add a scope like @knightcrawler/utility
, the same style applies to the other packages in the format @project/package
. Though it's not really required as just utilities
works too.
@sleeyax amazing thanks! JavaScript isn’t my primary language, so I’m going with the flow here 😂. This is a good idea. I’ll attempt this when I’m back online. |
@sleeyax is lib a common convention in these situations for JS? I was trying to decide what the best name would be |
@sleeyax you're more than welcome to fork and commit some changes and make a pull request 😆 |
Yep, was just about to do so @purple-emily you read my mind :) |
Yep. Workspaces is what we should use, rather than sub npm packages or github packages https://nx.dev/getting-started/intro It handles caching of build and test artifacts, less duplication of config files etc |
Ahh I hardly use it these days - you should see my unread message count hahaha I'm a .net dev really 23 years of it - this is the first JS project i've worked on in the past 4 years - although it seems not much has changed just tooling has gotten better ^^ |
Don't let me stop you 😂 I don't know what @sleeyax has done already |
Oh okay, I've used
Perfect. It's actually insane this project isn't using typescript already haha. |
Really is
Haha I hear ya - so frustrating the shared cache is just built in - gets stored in the .nx folder if we don't use nx cloud - so can just commit it |
Yeah I was surprised to see |
Haha - I only added recently - but that was part of the plan to move us forward. -plus we can still use watch, and pipe it into pino now i've added that too for pretty logs during dev |
Please consider moving moch's to a separate lib too, That way our only upstream (torrentio) commits to mochs for the time being will be to that lib - nothing else should hopefully have to change And once we have the moch's rewritten - we'll have to support those too Who knows - maybe Torrentio's author will end up pulling some of the improvements from us lol |
Ouch setting up I'll go ahead and create a PR for the NPM workspace restructure first so @purple-emily can continue moving shared code to the common lib. The See #60 |
Might be worth holding off on moving stuff to the shared codebase right now - The addon is 85% typescript in my pr draft now in #69 |
6adddf1
to
9f8f936
Compare
These `devDependencies` are no longer included in the child package (and should not be included anyways as they aren't imported).
Setup NPM workspace and shared devDependencies
This will close #56.
I'd like some help with this when anyones available, but I don't mind doing what I can for now. Ideally @iPromKnight you'll add me on Discord and help me quickly work out if I have the correct dev environment set up for JS 😂