-
Notifications
You must be signed in to change notification settings - Fork 48
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
Examples of relations in the docs are wrong for TS #162
Comments
Artaud
changed the title
Examples of relations in the docs are wrong
Examples of relations in the docs are wrong for TS
Nov 19, 2021
It would be better if you can open a PR, because I'm not sure that I fully
understand what you're looking for.
I'm not sure if you need to change the lib code or to add another example
of service & model initialization that works better with TypeScript.
בתאריך יום ו׳, 19 בנוב׳ 2021, 14:51, מאת Jiří Richter <
***@***.***>:
… Hi, I've been setting up a new Feathers app with ObjectionJS and
Typescript recently and found out that if I set up relations according to
the examples outlined in the docs here (on the feathers-objection lib), it
works initially - but once you start doing something less basic it falls
apart.
I think I have a correct way to set it up so if interested, I'll do a PR.
Unfortunately it's going to need a PR in the feathers generator as well
but it's not a big change so I'm keen on doing that as well if approved
here.
The main issue is that the docs here suggest doing the following for
setting up relations:
class User extends Model {
static get tableName() {
return 'user';
}
...
}
module.exports = function(app): typeof User {
if (app) {
const db = app.get('knex');
db.schema
.hasTable('user')
.then(exists => {
if (!exists) {
db.schema
.createTable('user', table => {
table.increments('id');
})
}
})
}
return User;
};
The problem is that in TS with the default export you can't return type
"User" but have to return "typeof User" since it's not an instance but a
class definition you're returning.
So I propose to also export the model class itself and drop the if (app)
check, so you'll use the default exported function only when creating the
Feathers service, but use the model export whenever you need to reference
it e.g. when setting up relations.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#162>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABB5E3IQTD7MRMRFB7E7THTUMZB5BANCNFSM5IMCHZOA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
It's just for docs here. I'll do the PR at the start of the next week. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi, I've been setting up a new Feathers app with ObjectionJS and Typescript recently and found out that if I set up relations according to the examples outlined in the docs here (on the feathers-objection lib), it works initially - but once you start doing something less basic it falls apart.
I think I have a correct way to set it up so if interested, I'll do a PR.
Unfortunately it's going to need a PR in the feathers generator as well but it's not a big change so I'm keen on doing that as well if approved here.
The main issue is that the docs here suggest doing the following for setting up relations:
The problem is that in TS with the default export you can't return type "User" but have to return "typeof User" since it's not an instance but a class definition you're returning.
So I propose to also export the model class itself and drop the
if (app)
check, so you'll use the default exported function only when creating the Feathers service, but use the model export whenever you need to reference it e.g. when setting up relations.The text was updated successfully, but these errors were encountered: