adopted-ember-addons / ember-changeset

Ember.js flavored changesets, inspired by Ecto
http://bit.ly/ember-changeset-demo
MIT License
431 stars 141 forks source link

Feature request to change constructor case to not break eslint "new-cap" rule #541

Closed rsmith-cs-ux closed 4 years ago

rsmith-cs-ux commented 4 years ago

Hi there!

When constructing a Changeset, the capitalization of the function doesn't play well with es-lint rules for new-cap seen here: https://eslint.org/docs/rules/new-cap.

this.changeset = Changeset(this.user, lookupValidator(userValidator)); // <-- Breaks the eslint rule

If it could be changed to be lowercase, or follow a standard constructor with the new keyword then this wouldn't need a special exception rule in the eslint config.

this.changeset = changeset(this.user, lookupValidator(userValidator)); // <-- Works no problem :)

Thanks!

snewcomer commented 4 years ago

Oh interesting! Never new of this rule. Does your lint rules run on dependencies as well?

rsmith-cs-ux commented 4 years ago

Apparently it does? Our initial reaction when seeing it was "how rude rule! We don't control that library." but I guess because we're calling the function within our code it doesn't like it ¯_(ツ)_/¯

JamesS-M commented 4 years ago

I've be using it like this.changeset = new Changeset(this.user, lookupValidator(userValidator)); without any issues. That should satisfy your lint rule!

rsmith-cs-ux commented 4 years ago

When we do that though, we get error TS7009: 'new' expression, whose target lacks a construct signature, implicitly has an 'any' type.. I could force the cast, but that doesn't feel right either.

JamesS-M commented 4 years ago

Oh I see, sorry I misunderstood the problem 😄

snewcomer commented 4 years ago

import { Changeset as changeset } from 'ember-changeset';

How about this?

rsmith-cs-ux commented 4 years ago

That is another workaround. I locally have added the exception to our lint rule but I was thinking it would be better to handle it at this level for others. If I'm the only one hitting the issue though I'm happy to leave it alone :)

snewcomer commented 4 years ago

Happy to provide another solution alongside exporting Changeset func (or accept a PR for one that effectively exports but calls original Changeset function). How common is this lint rule?

snewcomer commented 4 years ago

Happy to accept a PR reexporting a changeset function. Closing for now.