IIC2513-2017-2 / template

0 stars 0 forks source link

Feature: Sequelize #8

Closed wachunei closed 7 years ago

wachunei commented 7 years ago

Hola, en la rama feature-sequelize estuve jugando con sequelize, les quiero hacer un resumen de lo que aprendí en este rato:

negebauer commented 7 years ago

El archivo src/models/index.js es muy de configuración de la DB, y exporta el db. Debería ir en src/db/index.js También importarlo en src/app.js Esos son los primeros comentarios que se me vienen a la mente Y evitar tanto callback 😟

wachunei commented 7 years ago

Lo que pasa es que sequelize lo genera así al hacer sequelize init, y lo genera ahí porque ahí le dije (en el .sequelizerc) que models era el folder de los modelos. Y sobre los callback: así los genera sequelize, casi todos los cambios hasta ahora son generados por sequelize. 🤷‍♂️

raulmt commented 7 years ago

@wachunei primer comentario que se me viene a la cabeza: "invocar a @raulmt para que nos cuente cómo se usa"? Yo jamás en la vida he usado sequelize :) ya… ahora miraré el resto…

raulmt commented 7 years ago

respecto a models/index.js… es cierto que tiene mezcladas la conexión de BD junto a carga de modelos… pero como dijo @wachunei es como sequelize genera el archivo… y es más, es parte de cómo ellos proponen que se use sequelize con express. Quizá, después de todo, nuestra carpeta db es más de viudos de Rails que de otra cosa… porque efectivamente es raro tener una carpeta db y luego iniciar la conexión de db en el index de otra carpeta. Una opción sería seguir algo más cercano a lo que hacen en ese repo y tener models, migrations y seeds directamente en src. Quizá la config de la base de datos podría estar en src/config/database.js, pues igual me molesta que sólo tenga config de database y se llame simplemente "config"…

negebauer commented 7 years ago

Me gusta lo último que dijo @raulmt, matar la carpeta db

raulmt commented 7 years ago

Apliqué los cambios que mencioné en la revisión, pero no agregué koa-orm, pues es medio chanta ese package. En lugar de esto último, dejé el setup con sequelize directo como lo estaba haciendo @wachunei y agregué una property orm al ctx con un pequeño middleware.

Además, hice la config un poco más amigable, con un js que carga los datos de env variables.

Ojo que creo que necesitaremos hacer un fork de sequelize-cli y usar nuestro fork, pues la versión actual genera cosas de una forma que no se alínea ni con nuestro linter y, peor aún, ni con la versión de sequelize que estamos usando (v4), pues crea modelos con un atributo classMethods que ya no es soportado en sequelize v4. Pero creo que eso queda para otro PR :\