search

Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
26.88k stars 3.83k forks source link

Pluralize issue: 'vertex' -> 'vertexes' #11515

Closed justinlottes closed 1 year ago

justinlottes commented 2 years ago

Do you want to request a feature or report a bug? BUG

What is the current behavior? pluralize('vertex') === 'vertexes'

If the current behavior is a bug, please provide the steps to reproduce.

What is the expected behavior? pluralize('vertex') === 'vertices'

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

Uzlopak commented 2 years ago

Relevant code: https://github.com/Automattic/mongoose/blob/f148e629ae2c110c3797902db66f240cd4ddc771/lib/helpers/pluralize.js#L25

I think the regex is wrong. Instead of /(matr|vert|ind)ix|ex$/gi should be /(matr|vert|ind)(ix|ex)$/gi and then probably a line higher as the above line handles words ending on x.

Tests should be something like

six => sixes sex => sexes vertex => vertices Index => indizes

But I want to remind that changing this behavior can have catastrophic effects in production. It means that mongoose could try to check if a collection exists and then because the pluralization was changed does not find the production collection and then creates a new collection and creates new documents there.

I would rather recommend to manually set the collectionName and if we think pluralization should be fixed do it in mongoose v7.

vkarpov15 commented 2 years ago

We'll make this change for Mongoose 7. But, in the meantime, just do mongoose.model('Vertex', vertexSchema, 'vertices') to override pluralization for this case

vkarpov15 commented 1 year ago

After further review, I think we should leave the plural of "vertex" as "vertexes" to be consistent with the discussion from this issue: https://github.com/Automattic/mongoose/pull/11953 . TLDR; we argued that "indexes" is an acceptable plural form of "index" because major dictionaries like Oxford and Merriam-Webster agree. "vertexes" is sufficiently similar, and major dictionaries do list "vertexes" as an acceptable plural form of "vertex".

I'm going to close this as a won't fix. If you prefer vertices, changing it is a one-liner mongoose.model('Vertex', vertexSchema, 'vertices').