thinky icon indicating copy to clipboard operation
thinky copied to clipboard

Circular Dependency Issue with ES2015 / Babel 6

Open dacodekid opened this issue 8 years ago • 16 comments

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.

dacodekid avatar Dec 01 '15 16:12 dacodekid

It's probably the same issue as https://github.com/neumino/thinky/issues/351

neumino avatar Dec 01 '15 16:12 neumino

@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 avatar Dec 01 '15 16:12 dacodekid

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

neumino avatar Dec 01 '15 16:12 neumino

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

dacodekid avatar Dec 01 '15 17:12 dacodekid

shouldn't you be using:

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

since you aren't exporting Project as the default?

ondreian avatar Dec 01 '15 17:12 ondreian

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

dacodekid avatar Dec 01 '15 17:12 dacodekid

@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.

ondreian avatar Dec 01 '15 17:12 ondreian

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 avatar Dec 02 '15 06:12 neumino

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

neumino avatar Dec 02 '15 06:12 neumino

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 avatar Dec 02 '15 12:12 dacodekid

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 avatar Dec 02 '15 15:12 dacodekid

I ended up writing

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

But I might like your solution more

erik-singleton avatar Dec 05 '15 00:12 erik-singleton

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 avatar May 29 '16 13:05 samkelleher

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 avatar Aug 18 '16 17:08 evenfrost

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

neumino avatar Aug 20 '16 23:08 neumino

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 avatar Aug 21 '16 16:08 evenfrost