ilonatommy / reactWithDotnetOnWebWorker

1 stars 2 forks source link

feedback #1

Open pavelsavara opened 8 months ago

pavelsavara commented 8 months ago

nice work on the ASCII diagrams!

@maraf please review as well

ilonatommy commented 8 months ago

Yes, when running the app locally. What about leaving the file commented out with an info to uncomment for running on the localhost?

pavelsavara commented 8 months ago

Yes, that would work. Is that https://create-react-app.dev/docs/proxying-api-requests-in-development/ ? I don't see http-proxy-middleware in package.json is it included by default ?

ilonatommy commented 8 months ago

It seems I tested it wrong (not on clean build). I cannot reproduce any failures for localhost serving, I'll keep it removed. And yes, it was the mechanism from the link and the package should be required for that approach.

maraf commented 8 months ago

Otherwise it looks great!

pavelsavara commented 8 months ago

this is not Blazor, Ilona also has Blazor demo in progress

yes, currently there is

let uint8Array = new Uint8Array(e.data.image);
                let binaryString = Array.from(uint8Array).map(byte => String.fromCharCode(byte)).join('');
                let base64String = btoa(binaryString);
                document.getElementById("qrImage").src = `data:image/bmp;base64, ${base64String}`;

but we could possibly have just createObjectURL

const arrayBuffer = await imageStream.arrayBuffer();
    const blob = new Blob([arrayBuffer]);
    const url = URL.createObjectURL(blob);
    const image = document.getElementById(imageElementId);
ilonatommy commented 8 months ago

After the discussion offline I agree that we should stick to design: react only for frontend, WASM only for backend. My motivation of setting img source from WASM was purely to show that it's already possible from there but @maraf is right - it does not have any real value other than a fun-fact and won't be used that way. Both feedbacks are fully applied now.