search

expressjs / response-time

Response time header for node.js
MIT License
477 stars 73 forks source link

Warning on build for use of eval #22

Closed AlexBroadbent closed 2 months ago

AlexBroadbent commented 2 years ago

Version: 2.3.2

(!) Use of eval is strongly discouraged
https://rollupjs.org/guide/en/#avoiding-eval
../../../node_modules/response-time/node_modules/depd/index.js
408: 
409:    // eslint-disable-next-line no-eval
410:   var deprecatedfn = eval('(function (' + args + ') {\n' +
                          ^
411:     '"use strict"\n' +
412:     'log.call(deprecate, message, site)\n' +
carpasse commented 4 months ago

The use of eval comes from the node-depd dependency https://github.com/expressjs/response-time/blob/master/index.js#L16 which adds the deprecation warning message.

~@dougwilson @UlisesGascon version 2.0 of depd stops using eval. Is it worth updating the library? I can prepare a fix for it if you want.~

~Also if the package is deprecated I think it should be marked as so in npm with npm-deprecate~

Update I misunderstood when I quickly read the code. It is not the package that is deprecated but and old way of passing the digits option. I've prepared a PR to update the dependency

UlisesGascon commented 4 months ago

I am happy to do a release for it if you prepare the PR with the upgrade/removal and an additional PR to migrate from Travis to GitHub Actions.

Otherwise, we can deprecate the package. It seems like the package is still in use with 366,737 downloads per week.