protobuf.js icon indicating copy to clipboard operation
protobuf.js copied to clipboard

Get the package into a working state again

Open dcodeIO opened this issue 3 years ago • 5 comments

While proper modernization seems due, this PR for now tries to get all the things working again.

  • Updates dependencies but doesn't go to lengths in regards of breaking dependency changes yet.
  • Continued work towards separation of the protobufjs and protobufjs-cli packages.
  • Removes dist/ files from source control.
  • Updates CI scripts, drops testing on Node.js < 12 and sets up a new automated publishing step.

dcodeIO avatar Apr 07 '22 03:04 dcodeIO

Can you do something about #1704 while you're in there?

Qard avatar Apr 08 '22 16:04 Qard

I think this should be fixed by the dependencies update. The referenced PR doesn't do this I believe, since if package-lock is honored, there is no change as it hasn't been updated, and if it is not, the most recent ^7 will be installed anyway.

dcodeIO avatar Apr 09 '22 07:04 dcodeIO

Not updating the bottom end of a caret range in package.json means lower numbers are still considered valid. If npm tries to dedupe and flatten dependencies it may pick an older version to be in-range of all depending modules. Generally the package.json should be updated in the case of security vulnerabilities to explicitly block npm from allowing the module to depend on vulnerable versions of the dependency.

Qard avatar Apr 10 '22 06:04 Qard

TIL, thanks :)

dcodeIO avatar Apr 10 '22 10:04 dcodeIO

Anything we can do to help push this along? I'm interested in getting access to https://github.com/protobufjs/protobuf.js/commit/af1b449602b360091e191a58abde2f246d8b0f1d which fixes an option parsing error I'm experiencing. I'd also be interested in providing a fix to parse EnumValueOptions: https://github.com/protobufjs/protobuf.js/issues/1631

Thanks!

craigmarker avatar Jun 01 '22 22:06 craigmarker