mickhansen / graphql-sequelize

GraphQL & Relay for MySQL & Postgres via Sequelize
MIT License
1.9k stars 172 forks source link

instanceof check with npm link causes problems due to multiple versions of sequelize #633

Closed intellix closed 5 years ago

intellix commented 5 years ago

So I've recently had to transition from yarn/lerna (de-duped deps between shared packages) to separate packages and npm (due to separate service deployment complexities).

We've got 3 separate packages:

Both api and worker depend on core. All 3 depend on sequelize@4.39.0 because they're all using it directly.

After using npm link to share a local version of core into say api we're getting the error: primaryKeyAttribute of null and have tracked it down to this instanceof check:

https://github.com/mickhansen/graphql-sequelize/blob/ee0cde07be4db6856a42179dee4de9302373278c/src/relay.js#L24

When you npm link you also get the node_modules included and you end up with 2x versions of the same library. core -> sequelize@4.39.0 !== api -> sequelize@4.39.0.

It seems the easiest fix is to replace the instanceof check for a duck-type check. Sequelize Typescript check the sequelize['version'] param and then parseInt it: https://github.com/RobinBuschmann/sequelize-typescript/blob/master/lib/utils/versioning.ts#L11

intellix commented 5 years ago

actually it seems impossible as the library is riddled with instanceof checks :) https://github.com/mickhansen/graphql-sequelize/blob/b92521c1aa002e0a82b5101b46b3d0d90af0e404/src/typeMapper.js#L67

intellix commented 5 years ago

It seems the correct fix was to move sequelize to a peerDependency. It didn't work originally because I was also moving sequelize-typescript so it wouldn't compile.

Also the package which depends on it also needed --preserve-symlinks

mickhansen commented 5 years ago

duck type checking would indeed be better, more practical