modali icon indicating copy to clipboard operation
modali copied to clipboard

Modali uses shortid & nanoid at the same time

Open jaxxreal opened this issue 6 years ago • 2 comments

Hello!

That lib looks really promising, so thank you!

I found an issue exploring modali with bundlephobia.

From bundlephobia stats, it turns that modali uses shortid & nanoid in the same time. A purpose of these libs is the same, but shortid is 33% of the modali bundle, which is huge.

Снимок экрана 2019-07-24 в 11 06 12

jaxxreal avatar Jul 24 '19 08:07 jaxxreal

shortid uses nanoid, that is why both show up in bundlephobia. shortid is considered a dead project (the current maintainer states this multiple times in the issues and PRs), and nanoid is considered a better alternative.

In addition, the way the random generator is used in this project is a bit troublesome, as it generates new IDs for every render of the footer.

nanoid has a comment about this:

Do not use a nanoid for key prop. In React key should be consistence between renders. This is bad code:

<Item key={nanoid()} /> /* DON’T DO IT */

This is good code. this.id will be generated only once:

  id = nanoid()
  render () {
    return <Item key={this.id}>;
  }
}

mikaello avatar Oct 15 '19 14:10 mikaello

I guess nanoid doc is trying to protect React newcomers. Taking into account, that you change key each render intentionally, it doesn't look like a big deal to throw out shortid and use nanoid only. I can author a PR if needed.

jaxxreal avatar Oct 15 '19 15:10 jaxxreal