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

Support lfortran.wasm and lfortran.js of different commits #24

Conversation

Shaikh-Ubaid
Copy link
Member

@Shaikh-Ubaid Shaikh-Ubaid commented Aug 17, 2022

This PR adds support of using lfortran.wasm and lfortran.js of different merge commits of lfortran master.

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft August 17, 2022 21:28
@Shaikh-Ubaid Shaikh-Ubaid force-pushed the commit_hash_and_download_from_external3 branch from 636a014 to a14d78f Compare August 17, 2022 22:05
@Shaikh-Ubaid
Copy link
Member Author

Shaikh-Ubaid commented Aug 17, 2022

At the moment, I am unsure if the changes in this PR would work. Up-till now the changes in this PR were working fine locally on my system. In the past half an hour, the requests for lfortran.js and lfortran.wasm started failing because of CORS (Cross-origin resource sharing) errors.

The exact error is:

Access to fetch at 'https://ubaidshaikh.me/wasm_builds/commit1/lfortran.wasm' from origin 'http://localhost:8000' has been 
blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response 
serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

I am still hoping that the changes in this PR could/would work when deployed as after deployment the domains for both the live site dev.lfortran.org and the lfortran.wasm storage site lfortran.org/wasm_builds are/will-be the same.

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as ready for review August 17, 2022 22:07
@certik
Copy link
Contributor

certik commented Aug 18, 2022

I think the wasm file must be present in the same gh-pages branch as the rest of the site, and consequently available from dev.lfortran.org. I would not load it from elsewhere. See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS for more information.

@certik
Copy link
Contributor

certik commented Aug 18, 2022

To move forward, why not first implement just the latest wasm binary, copy it to gh-pages etc. Let's get it done, merged, etc.

After that, we can investigate how to support more versions.

@Shaikh-Ubaid
Copy link
Member Author

I just setup a demo of this PR here https://www.ubaidshaikh.me/lcompilers_frontend/. (The view button of each commit is not working perfectly, but it can fixed later).

@Shaikh-Ubaid
Copy link
Member Author

I think the wasm file must be present in the same gh-pages branch as the rest of the site, and consequently available from dev.lfortran.org. I would not load it from elsewhere. See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS for more information.

Got it.

To move forward, why not first implement just the latest wasm binary, copy it to gh-pages etc. Let's get it done, merged, etc.
After that, we can investigate how to support more versions.

Got it. I am marking this PR as draft. I will continue this later.

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft August 18, 2022 09:20
@certik
Copy link
Contributor

certik commented Aug 18, 2022

The demo looks very nice.

@Shaikh-Ubaid
Copy link
Member Author

I think the wasm file must be present in the same gh-pages branch as the rest of the site, and consequently available from dev.lfortran.org. I would not load it from elsewhere. See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS for more information.

It seems GitHub Pages support CORS requests (reference: https://stackoverflow.com/a/26417091/11474769).

At the moment, I am unsure if the changes in this PR would work. Up-till now the changes in this PR were working fine locally on my system. In the past half an hour, the requests for lfortran.js and lfortran.wasm started failing because of CORS (Cross-origin resource sharing) errors.

The changes worked initially but later stopped working, because, I guess later I was not using the right domain (I was using https://ubaidshaikh.me, today it started working when I used https://www.ubaidshaikh.me).

@certik
Copy link
Contributor

certik commented Aug 18, 2022

It seems that CORS requires some setup and support, and even thought github might now support it, I think it is more robust if we can figure out a way for it to work from the same domain.

@Shaikh-Ubaid
Copy link
Member Author

It seems that CORS requires some setup and support, and even thought github might now support it, I think it is more robust if we can figure out a way for it to work from the same domain.

Got it. I am closing this for the moment.

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

Successfully merging this pull request may close these issues.

None yet

2 participants