protocolbuffers / protobuf-javascript

BSD 3-Clause "New" or "Revised" License
375 stars 67 forks source link

A problem about the callback for jspb.Map.forEach #163

Closed jinyunshaobing closed 1 year ago

jinyunshaobing commented 1 year ago

What I see:

Map<string, string>.forEach(callback: (entry: string, key: string) => void, thisArg?: {} | undefined): void

I saw the entry and key here, but where is the value? Map in JS is:

Map<string, string>.forEach(callbackfn: (value: string, key: string, map: Map<string, string>) => void, thisArg?: any): void
dibenede commented 1 year ago

I simplified the foreach usages that appear in map_test.js and it seems to be working as you expect (like a normal JS Map):

const jspb  = require('google-protobuf');

const m = new jspb.Map([['hello', 1], ['world', 2]]);

m.forEach((v, k, whole) => {
  console.log('key=', k, 'value=', v);
});

// Prints:
// key= hello value= 1
// key= world value= 2

Do you have an example that's giving you an unexpected result?

jinyunshaobing commented 1 year ago

The result is right, but why the code prompt here in VSCode is error? VSCode shows:

Map<string, string>.forEach(callback: (entry: string, key: string) => void, thisArg?: {} | undefined): void

But the entry here should be value in fact.

dibenede commented 1 year ago

I'm not sure. Do you have any extra npm packages installed to provide type information?

Our closure types appear ok:

 * @param {function(this:T, V, K, ?jspb.Map<K, V>)} cb
jinyunshaobing commented 1 year ago

I find this in node_modules/@types/google-protobuf/index.d.ts At around line 190

forEach(
    callback: (entry: V, key: K) => void,
    thisArg?: {}): void;
dibenede commented 1 year ago

Ah, it looks like that project just happens to name the value field as entry, which could be a bit confusing. However, we don't have any relationship to the DefinitelyTyped project's types. You may want to file a bug/feature request to rename the parameter.