Skip to content

Declarations mismatch #102

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

Open
qt-kaneko opened this issue Feb 16, 2025 · 6 comments
Open

Declarations mismatch #102

qt-kaneko opened this issue Feb 16, 2025 · 6 comments

Comments

@qt-kaneko
Copy link

sqlite3.vfs.installVfs({
  io: {
    methods: {
      //                    \/ Should be bigint as bigint is actually passed
      xRead(fid, dest, n, offset) {
        ...
      }
    }
  }
})

Not sure about these, but

// Are named without $ in declarations, but it does not work without $

let vfs = new sqlite3.capi.sqlite3_vfs();
vfs.$iVersion = ...;
vfs.$mxPathname = ...;
vfs.$zName = ...;
vfs.$szOsFile = ...;

// And

let file = new sqlite3.capi.sqlite3_file();
file.$pMethods = ...;

Also there is no capi.sqlite3_file.structInfo in declarations, would be nice to have it.

@tomayac
Copy link
Collaborator

tomayac commented Feb 18, 2025

Closing here as per the warning in the README, but @sgbeal FYI.

@tomayac tomayac closed this as completed Feb 18, 2025
@sgbeal
Copy link
Collaborator

sgbeal commented Feb 18, 2025

i looked at this when it was initially posted. The upstream project has nothing to do with "declarations" and type info - those are typescript-isms (or non-vanilla-JS-isms, in any case).

@tomayac tomayac reopened this Feb 18, 2025
@tomayac
Copy link
Collaborator

tomayac commented Feb 18, 2025

Oh, I see that now, sorry.

So for the first point, this seems to be declared as number here:

iOfst: number,

The second point seems to be declared here:

sqlite-wasm/index.d.ts

Lines 1652 to 1657 in 870dca9

iVersion: number;
szOsFile: number;
mxPathname: number;
pNext: WasmPointer;
zName: WasmPointer;
pAppData: WasmPointer;

@qt-kaneko, since you seem to have looked into this already, are you in a position to suggest fixes?

@drozycki drozycki mentioned this issue Apr 4, 2025
@drozycki
Copy link
Contributor

drozycki commented Apr 4, 2025

@tomayac I came across this issue as well, and forked the repo with my fixes. PR here: #107

There's undoubtedly more examples of this missing $ but I couldn't find an easy way to find them all besides using all the APIs and noting where the expected behavior does not happen. Ideally the types would be generated upstream when the C types (or generated Emscripten types) are still known, but I couldn't find a way to do that either.

@drozycki
Copy link
Contributor

drozycki commented Apr 4, 2025

Not to mention that the symbols without the $ would be preferable for the sake of consistency with the more-detailed C API docs.

@sgbeal
Copy link
Collaborator

sgbeal commented Apr 4, 2025

The $ are an artifact of the C-struct-to-JS wrapper. We "need" those in JS so someone reading/writing the code (🙋‍♂️) can unambiguously ensure that they're dealing with C-struct members instead of JS object properties. Without those $ it's simply too easy to confuse C struct members with JS properties.

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

4 participants