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

Circular Dependency Issue with ES2015 / Babel 6 #399

Open
dacodekid opened this Issue Dec 1, 2015 · 16 comments

Comments

Projects
None yet
6 participants
@dacodekid

dacodekid commented Dec 1, 2015

Following thinky's recommended architecture,

\\ project.js
import { thinky, type } from 'app/util/thinky';

const Project = thinky.createModel('Project', {
  id: type.string(),
  name: type.string(),
  statusId: type.string()
});

export { Project };

import Status from 'app/models/lookup/status';
Project.belongsTo(Status, 'status', 'statusId', 'id');
\\ status.js
import { thinky, type } from 'app/util/thinky';

const Status = thinky.createModel('Status', {
  id: type.string(),
  name: type.string()
});

export { Status };

import Project from 'app/models/core/project';
Status.hasMany(Project, 'projects', 'id', 'statusId');

I'm getting throw new Error("First argument ofhasManymust be a Model") error. Converting this code with require works fine. It only produces the error if I add the relationship. So I couldn't come to a conclusion that it is babel's issue either. Would appreciate your input.

@neumino

This comment has been minimized.

Show comment
Hide comment
@neumino

neumino Dec 1, 2015

Owner

It's probably the same issue as #351

Owner

neumino commented Dec 1, 2015

It's probably the same issue as #351

@dacodekid

This comment has been minimized.

Show comment
Hide comment
@dacodekid

dacodekid Dec 1, 2015

@neumino, I saw that b4 posting but couldn't relate the error I'm receiving. I even tried to move all the relationships to another js file, but that didn't help either. It works only if I place all my models into a single file.

dacodekid commented Dec 1, 2015

@neumino, I saw that b4 posting but couldn't relate the error I'm receiving. I even tried to move all the relationships to another js file, but that didn't help either. It works only if I place all my models into a single file.

@neumino

This comment has been minimized.

Show comment
Hide comment
@neumino

neumino Dec 1, 2015

Owner

Can you provide a full example with 2 models that raises this error?

Owner

neumino commented Dec 1, 2015

Can you provide a full example with 2 models that raises this error?

@dacodekid

This comment has been minimized.

Show comment
Hide comment
@dacodekid

dacodekid Dec 1, 2015

@neumino , just pushed it. Plz take a look at this repo. https://github.com/dacodekid/hpa

dacodekid commented Dec 1, 2015

@neumino , just pushed it. Plz take a look at this repo. https://github.com/dacodekid/hpa

@ondreian

This comment has been minimized.

Show comment
Hide comment
@ondreian

ondreian Dec 1, 2015

Contributor

shouldn't you be using:

import { Project } from 'app/models/core/project';

since you aren't exporting Project as the default?

Contributor

ondreian commented Dec 1, 2015

shouldn't you be using:

import { Project } from 'app/models/core/project';

since you aren't exporting Project as the default?

@dacodekid

This comment has been minimized.

Show comment
Hide comment
@dacodekid

dacodekid Dec 1, 2015

@ondreian that didn't fix either. I was trying to import every way think of.

dacodekid commented Dec 1, 2015

@ondreian that didn't fix either. I was trying to import every way think of.

@ondreian

This comment has been minimized.

Show comment
Hide comment
@ondreian

ondreian Dec 1, 2015

Contributor

@dacodekid yeah, sorry bud. was just the first thing I saw that looked like it might have been the problem without looking at your app architecture. Hope you figure it out.

Contributor

ondreian commented Dec 1, 2015

@dacodekid yeah, sorry bud. was just the first thing I saw that looked like it might have been the problem without looking at your app architecture. Hope you figure it out.

@neumino

This comment has been minimized.

Show comment
Hide comment
@neumino

neumino Dec 2, 2015

Owner

I don't use es6 stuff, but after skimming over the docs, it seems that imports are hoisted (and are actually bindings), so this doesn't work.
I don't know what the standard way to work around, but I guess you can export a function that will create the relation and call that function at the appropriate place. Or you can just use require.

Owner

neumino commented Dec 2, 2015

I don't use es6 stuff, but after skimming over the docs, it seems that imports are hoisted (and are actually bindings), so this doesn't work.
I don't know what the standard way to work around, but I guess you can export a function that will create the relation and call that function at the appropriate place. Or you can just use require.

@neumino neumino closed this Dec 2, 2015

@neumino neumino added the question label Dec 2, 2015

@neumino neumino reopened this Dec 2, 2015

@neumino

This comment has been minimized.

Show comment
Hide comment
@neumino

neumino Dec 2, 2015

Owner

Reopening I'll write some docs later. I need to properly test thing though

Owner

neumino commented Dec 2, 2015

Reopening I'll write some docs later. I need to properly test thing though

@dacodekid

This comment has been minimized.

Show comment
Hide comment
@dacodekid

dacodekid Dec 2, 2015

I guess you can export a function that will create the relation and call that function at the appropriate place

That's what I exactly did at first. But then, wrapping the relationship inside model's ready promise seems to work. I haven't test the code, but at least I'm not getting the error. If you think this might be a workaround, plz close this issue.

UPDATE: Spoke too soon I guess. The below code doesn't seems to work. Need more test.

import {
  thinky, type
}
from 'app/util/thinky';

const Status = thinky.createModel('Status', {
  id: type.string(),
  name: type.string()
});

Status.ready().then(() => {
  Status.hasMany(thinky.models.Project, 'projects', 'id', 'statusId');
});

export {
  Status
};
import {
  thinky, type
}
from 'app/util/thinky';

const Project = thinky.createModel('Project', {
  id: type.string(),
  name: type.string()
});

Project.ready().then(() => {
  Project.belongsTo(thinky.models.Status, 'status', 'statusId', 'id');
});

export {
  Project
};

dacodekid commented Dec 2, 2015

I guess you can export a function that will create the relation and call that function at the appropriate place

That's what I exactly did at first. But then, wrapping the relationship inside model's ready promise seems to work. I haven't test the code, but at least I'm not getting the error. If you think this might be a workaround, plz close this issue.

UPDATE: Spoke too soon I guess. The below code doesn't seems to work. Need more test.

import {
  thinky, type
}
from 'app/util/thinky';

const Status = thinky.createModel('Status', {
  id: type.string(),
  name: type.string()
});

Status.ready().then(() => {
  Status.hasMany(thinky.models.Project, 'projects', 'id', 'statusId');
});

export {
  Status
};
import {
  thinky, type
}
from 'app/util/thinky';

const Project = thinky.createModel('Project', {
  id: type.string(),
  name: type.string()
});

Project.ready().then(() => {
  Project.belongsTo(thinky.models.Status, 'status', 'statusId', 'id');
});

export {
  Project
};
@dacodekid

This comment has been minimized.

Show comment
Hide comment
@dacodekid

dacodekid Dec 2, 2015

This is the only workaround I found. Moving on with this until find a better one.

import {
  thinky, type
}
from 'app/util/thinky';

const Project = thinky.createModel('Project', {
  id: type.string(),
  name: type.string()
});

Project.relationship = () => {
  Project.belongsTo(thinky.models.Status, 'status', 'statusId', 'id');
};

export {
  Project
};

dacodekid commented Dec 2, 2015

This is the only workaround I found. Moving on with this until find a better one.

import {
  thinky, type
}
from 'app/util/thinky';

const Project = thinky.createModel('Project', {
  id: type.string(),
  name: type.string()
});

Project.relationship = () => {
  Project.belongsTo(thinky.models.Status, 'status', 'statusId', 'id');
};

export {
  Project
};
@sasucker

This comment has been minimized.

Show comment
Hide comment
@sasucker

sasucker Dec 5, 2015

I ended up writing

let Status = require('./Status').default;
User.hasMany(Status, 'statuses', 'id', 'statusId');

But I might like your solution more

sasucker commented Dec 5, 2015

I ended up writing

let Status = require('./Status').default;
User.hasMany(Status, 'statuses', 'id', 'statusId');

But I might like your solution more

@samkelleher

This comment has been minimized.

Show comment
Hide comment
@samkelleher

samkelleher May 29, 2016

To jump in here, after using ES6 a lot and studying the proposed architecture from the docs, I've concluded that thinky is incompatible with new JavaScript module system and recommend using the require only.

This is due to the fact that the import statements are called immediately, so it's impossible to have two models depend on each other in a circular reference as simply importing them at startup will cause either one to fail because the other hasn't been created yet.

It also means that because the imports are called immediately, the model creation requires an instance of thinky, and because thinky connects to the database server as soon as it's created, this creates an undesirable situation where the application will actually begin establishing a connection during the initial tree setup of your application. So if you need to do any preparation or initialization in your application before establishing a database connection (like load config and connection settings), you can't.

samkelleher commented May 29, 2016

To jump in here, after using ES6 a lot and studying the proposed architecture from the docs, I've concluded that thinky is incompatible with new JavaScript module system and recommend using the require only.

This is due to the fact that the import statements are called immediately, so it's impossible to have two models depend on each other in a circular reference as simply importing them at startup will cause either one to fail because the other hasn't been created yet.

It also means that because the imports are called immediately, the model creation requires an instance of thinky, and because thinky connects to the database server as soon as it's created, this creates an undesirable situation where the application will actually begin establishing a connection during the initial tree setup of your application. So if you need to do any preparation or initialization in your application before establishing a database connection (like load config and connection settings), you can't.

@evenfrost

This comment has been minimized.

Show comment
Hide comment
@evenfrost

evenfrost Aug 18, 2016

So, what's the full scenario here to utilize Thinky with ES6 imports?
I've ended up with following:

// /utils/thinky.js
import Thinky from 'thinky';
export default new Thinky();

// /models/foo.js
import thinky from '../utils/thinky';

const { type, r } = thinky;

const {
  string,
  number,
  date,
} = type;

const Foo = thinky.createModel('Foo', {
  id: string(),
  name: string().required(),
  type: string(),
});

export { Foo };

// /anything.js
import { Foo } from './models/foo';
// use Foo model here

Are there any lines that could be optimized?

evenfrost commented Aug 18, 2016

So, what's the full scenario here to utilize Thinky with ES6 imports?
I've ended up with following:

// /utils/thinky.js
import Thinky from 'thinky';
export default new Thinky();

// /models/foo.js
import thinky from '../utils/thinky';

const { type, r } = thinky;

const {
  string,
  number,
  date,
} = type;

const Foo = thinky.createModel('Foo', {
  id: string(),
  name: string().required(),
  type: string(),
});

export { Foo };

// /anything.js
import { Foo } from './models/foo';
// use Foo model here

Are there any lines that could be optimized?

@neumino

This comment has been minimized.

Show comment
Hide comment
@neumino

neumino Aug 20, 2016

Owner

Use require? What does the ES6 import give you that require doesn't?

Owner

neumino commented Aug 20, 2016

Use require? What does the ES6 import give you that require doesn't?

@evenfrost

This comment has been minimized.

Show comment
Hide comment
@evenfrost

evenfrost Aug 21, 2016

For me this is possibility to use full ES6+ environment and have cleaner code as well. And following this logic, i.e. why not just use require, we could still write full ES5 and not bothering at all, but we all want to progress aren't we?

evenfrost commented Aug 21, 2016

For me this is possibility to use full ES6+ environment and have cleaner code as well. And following this logic, i.e. why not just use require, we could still write full ES5 and not bothering at all, but we all want to progress aren't we?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment