amuste / DnetIndexedDb

Blazor Library for IndexedDB DOM API
MIT License
105 stars 28 forks source link

Bug: Min / Max Empty Table #23

Closed Kylar182 closed 2 years ago

Kylar182 commented 3 years ago

I tried the new Min / Max Key / Index feature and it seems to have an issue when the table is empty. Says the Key is Null and throws an exception.

mcbride-clint commented 3 years ago

I was able to move the error up to the c# level, so the JS just returns null, but then System.Text.Json cannot deserialize null to string or int or whatever type you want.

Wrapping the call in a try/catch works and just return default whenever that happens. But I don't like relying on exceptions to guide the flow even though this should be a uncommon occurrence.

Making _jsRuntime.InvokeAsync return an JsonElement type works and can then check for ValueType.Null and return default.

Kylar182 commented 3 years ago

Any news on this?

mcbride-clint commented 3 years ago

Still awaiting the pull request to be approved. @amuste

amuste commented 3 years ago

I have already approved the pullrequest

Kylar182 commented 3 years ago

@mcbride-clint is the Observable / Async Code different on this method? It's not letting me call 2 queries simultaneously. It says the request didn't finish. I can reproduce it in the test repo if you'd like.

mcbride-clint commented 3 years ago

I was able to reproduce it I think. I assume you mean this:

    var indexMaxTask = GridColumnDataIndexedDb.GetMaxIndex<string>("TableFieldDtos", "attachedProperty");

    var indexMinTask = GridColumnDataIndexedDb.GetMinIndex<string>("TableFieldDtos", "attachedProperty");

    var indexMax = await indexMaxTask;

    var indexMin = await indexMinTask;

There shouldn't be anything different. I have never used RxJs so I just mimicked what I had seen in the other cursor range selects. There must be a variable or id that is making it use the same cursor when running more than one at a time.

mcbride-clint commented 3 years ago

I think I found the issue. Line 932

                    if (dbIndex) {
                        // Search By Index
                        const index = objectStore.index(dbIndex);
                        cursorRequest = index.openCursor(null, extentType);
                    } else {
                        // Search By Key
                        cursorRequest = objectStore.openCursor(null, extentType);
                    }

I never declared cursorRequest so it must be being hoisted by Javascript to be global and and being shared between the requests. It looks like putting let cursorRequest right above the line will fix it but I know @amuste changed all of his declarations from let to const in the past. I'm not super well versed in JS but is there any issue in using let?

noelrv commented 3 years ago
return new Rx.Observable((getReqObserver) => {
                        let cursorRequest = null;
                        if (dbIndex) {
                            // Search By Index
                            const index = objectStore.index(dbIndex);
                            cursorRequest = index.openCursor(null, extentType);

It makes sense to use let for cursorRequest; const can't be used as it is assigned a value in more than one occasion.

mcbride-clint commented 3 years ago

Thanks for the feedback. I have mostly used Internet Explorer so let and const are new to me. definitely an improvement over just var from what I can tell.