Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not able to assign a array inside a struct #17

Open
maneetgoyal opened this issue Jun 7, 2021 · 4 comments
Open

Not able to assign a array inside a struct #17

maneetgoyal opened this issue Jun 7, 2021 · 4 comments

Comments

@maneetgoyal
Copy link

Current Scenario:

import refNAPI from "ref-napi";
import StructType from "ref-struct-di";
import ArrayType from "ref-array-di";

const NAPIStructType = StructType(refNAPI);
const NAPIArrayType = ArrayType(refNAPI);

export const MAXVAL = 20;
export const NUM_H_ISOTOPES = 3;

export const inchi_Atom = NAPIStructType({
  x: refNAPI.types.double,
  y: refNAPI.types.double,
  z: refNAPI.types.double,
  neighbor: NAPIArrayType(refNAPI.types.short, MAXVAL),
  bond_type: NAPIArrayType(refNAPI.types.char, MAXVAL),
  bond_stereo: NAPIArrayType(refNAPI.types.char, MAXVAL),
  elname: NAPIArrayType(refNAPI.types.char, ATOM_EL_LEN),
  num_bonds: refNAPI.types.short,
  num_iso_H: NAPIArrayType(refNAPI.types.char, NUM_H_ISOTOPES + 1),
  isotopic_mass: refNAPI.types.short,
  radical: refNAPI.types.char,
  charge: refNAPI.types.char,
});

I am trying to instantiate an object of the above struct type:

const inchiAtom = new inchi_Atom({
  x: 1.1,
  y: 2.2,
  z: 3.3,
  neighbor: new NAPIArrayType(refNAPI.types.short)(MAXVAL),
  bond_type: new NAPIArrayType(refNAPI.types.char)(MAXVAL),
  bond_stereo: new NAPIArrayType(refNAPI.types.char)(MAXVAL),
  elname: new NAPIArrayType(refNAPI.types.char)(ATOM_EL_LEN),
  num_bonds: 3,
  num_iso_H: new NAPIArrayType(refNAPI.types.char)(NUM_H_ISOTOPES + 1),
  isotopic_mass: 10,
  radical: 2,
  charge: -2,
});

Getting the following error:

/workspaces/inchi/node_modules/ref-array-di/lib/array.js:189
    throw new Error('not sure how to set into Array: ' + value)
    ^

Error: not sure how to set into Array: 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
    at Function.set (/workspaces/inchi/node_modules/ref-array-di/lib/array.js:189:11)
    at Object.set (/workspaces/inchi/node_modules/ref-napi/lib/ref.js:492:10)
    at StructType.desc.set [as neighbor] (/workspaces/inchi/node_modules/ref-struct-di/lib/struct.js:224:16)
    at new StructType (/workspaces/inchi/node_modules/ref-struct-di/lib/struct.js:99:19)
    at Object.<anonymous> (/workspaces/inchi/dist/tsc/tests/examples.test.js:32:19)
    at Module._compile (node:internal/modules/cjs/loader:1109:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1138:10)
    at Module.load (node:internal/modules/cjs/loader:989:32)
    at Function.Module._load (node:internal/modules/cjs/loader:829:14)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
error Command failed with exit code 1.

Can anyone please suggest if this is a bug?

@maneetgoyal
Copy link
Author

If I try to instantiate via the following syntax, then it is working as expected:

const inchiAtom = new inchi_Atom({
  x: 1.1,
  y: 2.2,
  z: 3.3,
  // neighbor: new NAPIArrayType(refNAPI.types.short)(MAXVAL),
  // bond_type: new NAPIArrayType(refNAPI.types.char)(MAXVAL),
  // bond_stereo: new NAPIArrayType(refNAPI.types.char)(MAXVAL),
  // elname: new NAPIArrayType(refNAPI.types.char)(ATOM_EL_LEN),
  num_bonds: 3,
  // num_iso_H: new NAPIArrayType(refNAPI.types.char)(NUM_H_ISOTOPES + 1),
  num_iso_H: [1, 2, 3, 4, 5, 6],  // <===== Changed syntax
  isotopic_mass: 10,
  radical: 2,
  charge: -2,
});

But TSC throws type-mismatch error:

Type 'number[]' is missing the following properties from type 'TypedArray<number, number>': toArray, toJSON, inspect, buffer, refts(2769)

Hi @rbuckton, request you to please suggest something if possible. The type definitions of ref-array-di were recently updated here: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/ref-array-di/index.d.ts so thought you may some suggestions for us.

cc @KiranNiranjan @keerthi16 I will move the issue to DefinitelyTyped if it is type definition error instead of a bug in this library or in ref-array-di library.

@rbuckton
Copy link

rbuckton commented Jun 8, 2021

I'll take a look at the type definitions. I'm not an expert in ref-napi, but have been doing what I can to improve the type definitions. I was unaware that passing a vanilla JavaScript array would coerce properly. I may have to change the definition of ref-napi's Type to take in a "read" and a "write" type for coercion cases like this.

I'll note I've been able to use the ref-array-di ArrayType successfully as a receiver, like this:

const WORD = ref.types.uint16;
const LONG = ref.types.int32;
export const IMAGE_DOS_HEADER = StructType({
    e_magic:    WORD,
    e_cblp:     WORD,
    e_cp:       WORD,
    e_crlc:     WORD,
    e_cparhdr:  WORD,
    e_minalloc: WORD,
    e_maxalloc: WORD,
    e_ss:       WORD,
    e_sp:       WORD,
    e_csum:     WORD,
    e_ip:       WORD,
    e_cs:       WORD,
    e_lfarlc:   WORD,
    e_ovno:     WORD,
    e_res:      ArrayType(WORD, 4),
    e_oemid:    WORD,
    e_oeminfo:  WORD,
    e_res2:     ArrayType(WORD, 10),
    e_lfanew:   LONG,
});

But I haven't used it as an initialized value yet, so I could have missed something in the typings.

@rbuckton
Copy link

rbuckton commented Jun 8, 2021

I noticed that if you just elide the data fields, the struct is instantiated fine, since it uses defaults for the arrays:

const inchiAtom = new inchi_Atom({
  x: 1.1,
  y: 2.2,
  z: 3.3,
  num_bonds: 3,
  isotopic_mass: 10,
  radical: 2,
  charge: -2,
});

While I can investigate the type changes I made, it does seem like a possible bug that you cant assign a typed array instance? I'd have to let the project authors weigh in on that.

@maneetgoyal
Copy link
Author

maneetgoyal commented Jun 8, 2021

Many thanks for the response 👏

I was unaware that passing a vanilla JavaScript array would coerce properly.

Noticed that coercion happens properly with ref.type.char as well. For example:

export const ABCD = NAPIStructType({
  radical: refNAPI.types.char,
});

const instance = new ABCD({
  radical: "3",
});

strict.equal(ABCD.radical, "3".charCodeAt(0));  //  <===== assertion PASSES

While I can investigate the type changes I made, it does seem like a possible bug that you cant assign a typed array instance? I'd have to let the project authors weigh in on that.

Upon following the error stack, it let to ref-array-di source code:

/**
 * The "set" function of the Array "type" interface.
 * Most likely invoked when setting within a "ref-struct" type.
 */

function set (buffer, offset, value) {
  debug('Array "type" setter for buffer at offset', buffer, offset, value)
  var array = this.get(buffer, offset)
  var isInstance = value instanceof this
  if (isInstance || isArray(value)) {
    for (var i = 0; i < value.length; i++) {
      array[i] = value[i]
    }
  } else {
    throw new Error('not sure how to set into Array: ' + value)
  }
}

Here "value" is a buffer when we instantiate as described in the issue description, and that case seems to be unhandled (?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants