IIC2513-2021-1 / projects

Repositorio oficial para los proyectos de ambas secciones del periodo 2021-1 del curso PUC IIC2513
14 stars 6 forks source link

ESLint: Solicitudes para ignorar reglas #43

Open B-Dominguez opened 3 years ago

B-Dominguez commented 3 years ago

En esta issue podrán mostrar casos particulares en los que crean que deberían poder ignorar una regla de ESLint, para conseguir la confirmación y que no perjudique su nota.

Deben mostrar las líneas de código afectadas, la regla a ignorar y una justificación de por qué correspondería ignorar la regla.

vtunon7 commented 3 years ago

Hola! Queríamos ver si es que podemos ignorar las siguientes reglas porque nos tira error de ESLint de no-unused-variables y not defined en funciones como module, process, require, etc. las cuales están en archivos con código autogenerados como las seeds y migraciones.

WhatsApp Image 2021-05-18 at 13 08 59

Por lo tanto, queremos agregar las siguientes líneas en el archivo .eslintrc.json, específicamente en "env" ("node:" true) y "rules" ("no-unused-vars")

WhatsApp Image 2021-05-18 at 13 09 23

Entonces, al ser líneas de código autogeneradas, si las sacamos no correrá nuestra app

fbpesenti commented 3 years ago

Hola, tengo el mismo problema en las seeds y en las migraciones que me muestra error con la variable sequelize pero no la puedo borrar al ser autogenerada.

image

image image image

MFMG99 commented 3 years ago

Hola! Nosotros tenemos un problema Para hacer que la navbar (alojada en layout.html.ejs) cambie dependiendo de la ruta utilizamos _matchedRoute. ESLint nos tira el siguiente error: Captura de Pantalla 2021-05-19 a la(s) 11 23 06 Captura de Pantalla 2021-05-19 a la(s) 11 14 42 Le quitamos el guión bajo, ESLint nos dejó de tirar error, pero la navbar no funciona. Por eso queríamos ver si podíamos ignorarlo :c

mpmunoz13 commented 3 years ago

Hola! Queriamos saber si podemos ignorar el siguiente error. image Leyendo la documentación encontramos esto y queremos saber si aplica en este caso porque las operaciones que debemos hacer no son independientes. image

Zetaku1 commented 3 years ago

Tenemos un problema parecido, por lo que nos gustaría ignorar el error 'await inside a loop' image

dhvasquez commented 3 years ago

Hola, para el tema de "await inside a loop", si realmente no ven otra manera de solucionar su problema, hablenlo con sus ayudantes asignados porque pueden guiarlos en encontrar otra manera, o permitirles ignorar dicho error, porque depende mucho el como estan realizando sus loops. Pero, por lo menos en los casos que nos mostraron, yo diría que no pueden, ya que son loops que involucran búsquedas en bases de datos, y estamos utilizando PostgreSQL lo cual puede permitir busquedas con relaciones (recuerden de bases de datos que podemos hacer peticiones en SQL, juntando varias tablas y anidadonde queries, y sequelize tiene herramientas para traspasar ese estilo de peticiones desde javascript a SQL, como pueden verlo en las consolas).

Por ejemplo, el caso de @mpmunoz13 no sabría decir ya que no logro entender bien la finalidad de la función, pero lo mejor es que lo veas en detalle con tu ayudante asignado. Para el caso de @Zetaku1, por lo que tengo entendido, buscas los posts, y luego le quieres asignar sus usuarios respectivos, sin embargo, tu solución es poco "eficiente" (tampoco se va a demorar mucho pero no es recomendable), lo mejor es utilizar la función de include de sequelize, para que en vez de realizar muchas queries, solo se haga una query que incluya las relaciones respectivas, y venga "incluido" con cada instancia de post (al fin y al cabo esa es la ventaja de una base relacional, usar las relaciones). Pero con lo anterior dicho, tampoco se el scope completo de tu solución, talvez es un caso donde usar include es poco viable, por lo que también recomendaria que lo hables en detalle con tu ayudante asignado.

En resumen, hablen bien con sus ayudantes asignados, pues la respuesta de si pueden ignorarlo o no, depende mucho de que quieren lograr.

dhvasquez commented 3 years ago

@fbpesenti @MFMG99 Sus casos si pueden ser ignorados, ya que son casos particulares, pues:

@MFMG99 Tu caso es que "_algo" es una sintáxis que suele usarse para recursos privados, pero viene en el "ctx" asique no hay mucho que hacer 😆, puedes incluso ignorarlo para todo el archivo.

@fbpesenti El tuyo también, en general eslint te avisa sobre variables sin uso por temas de que no instancies cosas sin la necesidad de existir, pero es auto generado y no se puede borrar, asique también puedes ignorarlo para todo el archivo.

En general, cosas autogeneradas o ya instanciadas, si no lo pueden arreglar de ninguna manera, entonces ignorenlo.

dasolari commented 3 years ago

Hola! Complementando un poco lo que dice @dhvasquez , tal como dice en la foto que subió @mpmunoz13, un loop puede ser utilizado para prevenir que el código envíe una cantidad excesiva de requests. Ahora, los await ctx.orm.recurso... al ser operaciones que se hacen sobre una base de datos, son lentas comparadas con la ejecución de un loop, por lo que la probabilidad de que se generen inconsistencias es alta. Por esta razón existe ese error, ya que hay otras formas de hacer esa clase de operaciones. Recomiendo que revisen este link, investiguen sobre los métodos estáticos.

dhvasquez commented 3 years ago

@vtunon7 Puedes ignorar el error no-unused-variables para un archivo autogenerado, pero no lo agregues a la configuración, pues solo se permitira ignorar para archivos autogenerados.

Para los errores en module y require, te pediria si puedes mostrar con más detalle que error te dice, ya que no logro captar cual es el error de linting en esos casos.

Zetaku1 commented 3 years ago

Hola! me gustaría saber que se puede hacer con el siguiente error "no-shadow": image image

Debido a que esa arrow function es necesaria para que los path de los botones funcionen correctamente.

dhvasquez commented 3 years ago

@Zetaku1 es dificil ver el scope entero, pero si entiendo bien, en tu vista ejs, utilizas esa función para generar la ruta "edit" del post específico que buscaste, si es así y no utilizas esa arrow function para generar la ruta de algún otro post aparte del buscado, entonces puedes simplemente enviar la ruta ya generada, y utilizarla como variable.

PapayaManco commented 3 years ago

Hola, me gustaria saber por que sale el error de "no-undef" para esto: image

image

tomascortes commented 3 years ago

image

Hola, el lintern nos tira un error diciendo que el "\" es innecesario pero como es una url cuando lo sacamos no funciona. Podemos evitar que revise esa linea?

bpardobravo commented 3 years ago

Hola, el linter me tira el siguiente error y entendemos porqué está pasando, sin embargo nosotros necesitamos agregarle un nuevo atributo a comments sin crear una nueva variable.

Se puede ignorar o hay alguna manera de hacer esto mismo pero sin el error?

image

image

sivicencio commented 3 years ago

@PapayaManco Pareciera que ESLint no está reconociendo las funciones de Jest. ¿Por casualidad modificaste tu archivo .eslintrc en la raíz del proyecto? Como referencia, el de GudReads está de esta manera y el linter no tira los errores que mencionas.

@tomascortes No entiendo bien a qué te refieres con "". No lo veo en la línea de código que pegaste. ¿Podrías incluir el error asociado que tira ESLint?

@bpardobravo Cuando le intentas agregar la property username a la variable comment, estás haciendo una query por cada comentario para agregarle el usuario asociado. Eso es un problema conocido como n + 1 queries, porque haces una query para obtener los comentarios, y luego n queries, una por cada comentario. Eso se puede solucionar de forma simple con include en la llamada a findAll, y con eso no tendrías que hacer la asignación que te tira error. Mira la cápsula 12 donde se habla de esto.

Mjtala commented 3 years ago

@sivicencio Tengo el mismo error que @PapayaManco y revisé el archivo .eslintrc y lo tengo igual que en GudReads. :(

juanpacl111 commented 3 years ago

Hola! Le estoy pasando atributos adicionales a mis objetos obtenidos desde la base de dato para darle información adicional a la vista. Por ejemplo, a cada publicación (comment en mi caso) le doy un atributo comment.like para que la vista tenga esta información para cada comentario.

image image

image

Eslint me lanza no-param-reassign en cada instancia en que hago esto. Puedo ignorar ese error?

sivicencio commented 3 years ago

@Mjtala @PapayaManco No veo por qué podría tirarles error si tienen el archivo .eslintrc igual que en GudReads. Viendo el código, puedo suponer que el error está en src/routes/hello.test.js, que es un archivo de tests que está desde que uno crea el proyecto con el template. O bien les tiró error desde que se incorporó el uso de ESLint, o algo sucedió entre medio que hizo que comenzara a fallar. Les sugiero contactarse con su ayudante para ver el caso particular de forma más expedita.

@juanpacl111 Existe una manera de evitar el error si, en vez de iterar por comments y asignarle nuevas propiedades a cada elemento, creas un nuevo objeto producto de un map. Puedes incluso apoyarte con el uso de destructuring:

const augmentedComments = comments.map((comment) => {
  /*  const totalLikes = 'Get total likes' */
  return {
    ...comment,
    totalLikes,
    liked: false,
  };
});
tomascortes commented 3 years ago

@sivicencio perdón, me refería al carácter "\" el error es

4:35 error Unnecessary escape character: - no-useless-escape y la linea de codigo es

image

slira4 commented 3 years ago

@PapayaManco @Mjtala yo tenía el mismo problema! Era porque tenía un archivo .eslintrc.json además del archivo .eslintrc. Eliminé ese archivo y funcionó bien (también funciona agregar "plugin:jest/recommended" al extends del archivo .json)

sivicencio commented 3 years ago

@tomascortes No veo que necesiten escapar ese caracter, tal como indica ESLint. Y no entendí la parte en que dices "pero como es una url cuando lo sacamos no funciona". Como prueba, cambié la ruta de un nuevo libro en GudReads para que tenga un guión (/new-book) y funcionó sin problemas:

Screen Shot 2021-05-24 at 23 29 28

No deberías eliminar la regla de ESLint, sino resolver el problema con la ruta (que a priori no sé cuál puede ser sin tener más contexto).

Marialuisaclaro commented 3 years ago

Hola! Para la relación de una reunión con dos usuarios, hice dos relaciones belongsTo con un alias y un hasMany en el usuario Captura de pantalla 2021-06-12 a la(s) 13 24 04 Pero cuando creaba la reunión me salía el siguiente error: column "userId" does not exist. Lo solucioné agregando lo siguiente en usuario: Captura de pantalla 2021-06-12 a la(s) 13 23 52

Ahora el linter me tira el siguiente error: 15:66 error Duplicate key 'foreignKey' no-dupe-keys

¿Puedo ignorar esa regla? ¿O que puedo hacer para solucionarlo?

agustin-rios commented 3 years ago

Hola, me gustaria saber si puedo agregar lo siguiente: image image Esto para agregar labels, entre otros, o meter estructuras a los tags de la siguiente forma: image

sivicencio commented 3 years ago

@Marialuisaclaro No sé si aún persiste tu problema, pero responderé igualmente: el problema es porque básicamente estás especificando un objeto que tiene repetida la key foreignKey (y eso tiene que ver puramente con JavaScript). Para el contexto en que utilizaste esa key repetida, deberías ver cómo resolverlo tal vez con más de una sentencia this.hasMany, agregándole un nombre o alias a la asociación para diferenciar cada una (como en belongsTo).

@agustin-rios Sobre las primeras reglas que mencionas (la primera imagen), la mayoría tiene que ver con accesibilidad web. Es importante tener en cuenta esos temas en nuestras aplicaciones, por lo que te sugeriría resolverlos en vez de ignorarlos. Por ejemplo, tomando la regla jsx-a11y/click-events-have-key-events, puede ser que hayas utilizado un elemento con onClick que no es realmente clickeable (https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/master/docs/rules/click-events-have-key-events.md).

Sobre la regla react/jsx-props-no-spreading, puedes ignorarla si el paso de props de esa manera te facilita el desarrollo de forma considerable (por ejemplo, si le pasas más de 5 props a un componente y puedes obtenerlas de mejor manera a partir de un objeto al que le haces destructuring). Si vas a pasar menos de 5 props, en general es mejor hacerlo explícito, pues el mismo código asociado al render del componente queda más explícito.

MatiasDuhalde commented 3 years ago

Hola @sivicencio !

¿Podemos ignorar la regla react/button-has-type de eslint-plugin-react? (que es instalado por defecto por CRA). Tenemos un codigo parecido a lo siguiente:

const MyButton = (props) => {
  const { type } = props;
  return (
    // eslint-disable-next-line react/button-has-type
    <button className="..." type={type}>
      // ...
    </button>
  );
};

MyButton.propTypes = {
  type: PropTypes.oneOf(['submit', 'button', 'reset']),
};

MyButton.defaultProps = {
  type: 'button',
};

Sin el comentario para desactivar la regla, el linter da el siguiente error:

The button type attribute must be specified by a static string or a trivial ternary expression

Esto pasa porque el linter pide que el type del botón se asigne estáticamente (también puede ser definido por una expresión ternaria, pero los valores de la expresión también deben ser estáticos). Esto, para validar que esté dentro de los valores válidos para el type, que la MDN los define como "submit", "button", y "reset".

Para nuestro caso de uso, queremos pasarle al componente el type como prop, pero esto el linter lo reconoce como una asignación dinámica y lanza el error. Ya definimos la validación de la property abajo del componente, por lo que nos "evitamos" el caso de que se le pase un type inválido en las props.

En esta issue del plugin hay una discusión respecto a esta regla, y no existe un workaround directo.

https://github.com/yannickcr/eslint-plugin-react/issues/1555

Gracias!

sivicencio commented 3 years ago

@MatiasDuhalde Súper buen punto el que mencionas. Me parece bien, a diferencia de lo indicado por una de las personas en la discusión que mencionaste, crear un componente que permita hacer render de distintos tipos de botones. Además estás justamente manejando el caso de los valores posibles con propTypes.

Por lo anterior, puedes deshabilitar la regla sólo en ese componente (si usaras otro botón directamente como <button /> en otra parte de la aplicación, no debería estar ignorada la regla).