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

saveAsJSON() method not found #20

Closed
stbrody opened this issue Mar 1, 2021 · 10 comments
Closed

saveAsJSON() method not found #20

stbrody opened this issue Mar 1, 2021 · 10 comments
Labels

Comments

@stbrody
Copy link

stbrody commented Mar 1, 2021

The README says that you can call .saveAsJSON() on any bloom filter to get a serialization of the bloom filter state that can be persisted or sent over the network. When I try to use it, however, my Typescript code fails to compile with the error: error TS2339: Property 'saveAsJSON' does not exist on type 'BloomFilter'.

My code looks like:

import { BloomFilter } from 'bloom-filters'

// [...]
const bloomFilterEntries = new Set<string>()
// [...]

const bloomFilter = BloomFilter.from(bloomFilterEntries, 0.01)
const bloomJSON = bloomFilter.saveAsJSON()
@stbrody
Copy link
Author

stbrody commented Mar 1, 2021

When I look at the bloom-filter.d.ts file within my node_modules that was set up by NPM, I don't see a saveAsJSON method in the BloomFilter class. Similarly, looking at the bloom-filter.ts source file, I don't see one there either. So it makes sense that the node compiler would fail to find that method since it doesn't seem to exist.

I do however see some logic related to saveAsJSON in the exportable.ts file in the source. Is there something special I need to do to get my BloomFilter instance to register as an Exportable?

@Callidon
Copy link
Owner

Callidon commented Mar 2, 2021

The method exists for all classes in the package, but I think the latest rework might have removed the method from the definition files, which causes your TypeScript compiler to fail.

I can't promise that we can provide a fix quickly, because we now have very little time to work on open source projects (sadly). But I will definitively look into it, because that 's a rather nasty bug !

@Callidon Callidon added the bug label Mar 2, 2021
@folkvir
Copy link
Collaborator

folkvir commented Mar 7, 2021

@Callidon I think adding saveAsJSON and fromJSON into the base-filter as simple abstract/interface methods will solve our problem.
Those methods will be modified by the auto-exportable decorator anyway.

@folkvir
Copy link
Collaborator

folkvir commented Mar 7, 2021

@stbrody Please, can you test the following branch? https://github.com/Callidon/bloom-filters/tree/fix_saveasjson
Everything should work correctly.

@stbrody
Copy link
Author

stbrody commented Mar 10, 2021

@folkvir @Callidon, sorry I just got to looking at this again. I tried upgrading the package to version 1.3.2 and things are actually worse now. There's no longer a compilation error on saveAsJSON or fromJSON, but now any time I try to call the .has() method on a filter parsed out from fromJSON I get a new compilation error:

src/merkle/__tests__/merkle-metadata.test.ts:39:24 - error TS2339: Property 'has' does not exist on type 'void'.

Looks like because fromJSON doesn't specify a return type the TS compiler assumes it returns nothing. It probably needs to specify that it returns a BloomFilter. I assume a similar problem most likely exists for saveAsJSON() as well.

@folkvir
Copy link
Collaborator

folkvir commented Mar 12, 2021

Well, do you still have the issue with this PR? #22
The problem is that the method is overrided by a templated decorator which actually returns the correct object but is detected by the compiler as a void one. It should now be a Basefilter which can be easily casted as a BloomFilter as explained in the PR.
But it still requires you to cast the object in the right format.

@stbrody
Copy link
Author

stbrody commented Mar 13, 2021

Well, do you still have the issue with this PR? #22
The problem is that the method is overrided by a templated decorator which actually returns the correct object but is detected by the compiler as a void one. It should now be a Basefilter which can be easily casted as a BloomFilter as explained in the PR.
But it still requires you to cast the object in the right format.

Yeah, I guess that makes it a little better, but it would be nice not to have to cast at all. If BaseFilter actually had definitions of the common bloom filter functions (add, has, remove) that are shared by all the filter implementations then you could use what is returned from fromJSON directly without caring about what underlying filter type is being used.

@folkvir
Copy link
Collaborator

folkvir commented Mar 14, 2021

I agree with your point of a non-casting obligation but BaseFilter is a super class defining seeding properties of every bloom filters. It does not have any specific method. So I need to find another method dealing with decorators and templates.

@folkvir
Copy link
Collaborator

folkvir commented Mar 15, 2021

@Callidon @stbrody Unfortunately with our templating methods and decorators we cannot make a small change without redefining fromJSON in all Filters of the project. So in the last commit I defined the return type of fromJSON to any this way you can have type definitions only in two ways.

  • const b: BloomFilter = BloomFilter.fromJSON(bloomJSON)
  • const b = BloomFilter.fromJSON(bloomJSON) as BloomFilter

This is the only solution for us.

Working example:

import { BloomFilter } from '../../bloom-filters/dist/api'

// [...]
const bloomFilterEntries = new Set<string>(["test"])
// [...]

const bloomFilter: BloomFilter = BloomFilter.from(bloomFilterEntries, 0.01)
const bloomJSON: Object = bloomFilter.saveAsJSON()
console.log('JSON: ', bloomJSON)
const b: BloomFilter = BloomFilter.fromJSON(bloomJSON)
console.log('B: ', b)
console.log(b.has('test'))

PS: Just a reminder if you are not aware of it, don't even try to test or add data on a Filter that are initialized with an empty set or array. It will cause RangeError: Maximum call stack size exceeded. T.from functions are used to initialize filters with predefined values. Otherwise use the classic constructor or the T.create function to initialize a filter of fixed size with desired error rate.

@folkvir
Copy link
Collaborator

folkvir commented Mar 18, 2021

@stbrody You can upgrade to 1.3.3.
Thank you @Callidon

@folkvir folkvir closed this as completed Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants