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

Deno 1.7.0 incompatibility #92

Closed
FallingSnow opened this issue Jan 20, 2021 · 17 comments · Fixed by #28
Closed

Deno 1.7.0 incompatibility #92

FallingSnow opened this issue Jan 20, 2021 · 17 comments · Fixed by #28
Labels
✘ bug Something isn't working

Comments

@FallingSnow
Copy link
Contributor

FallingSnow commented Jan 20, 2021

$ aleph dev 
Check https://deno.land/x/aleph@v0.2.28/cli.ts
Check https://deno.land/x/aleph@v0.2.28/cli/dev.ts
error: Uncaught (in promise) NotSupportedError: Cannot set "location".
        throw new DOMException(`Cannot set "location".`, "NotSupportedError");
              ^
    at set (deno:op_crates/web/12_location.js:340:15)
    at Function.assign (<anonymous>)
    at Project._init (project.ts:600:16)
    at async project.ts:106:13
    at async start (server.ts:12:5)

$ deno --version
deno 1.7.0 (release, x86_64-unknown-linux-gnu)
v8 8.9.255.3
typescript 4.1.3

See denoland/deno#4981 (comment)

@shadowtime2000 shadowtime2000 added the ✘ bug Something isn't working label Jan 20, 2021
@FallingSnow
Copy link
Contributor Author

See for more info.

location: {

@FallingSnow
Copy link
Contributor Author

Possibly related changes to globalThis.location: https://deno.land/posts/v1.7#add-support-for-codeglobalthislocationcode-and-relative-fetch

@shadowtime2000
Copy link
Member

We have to figure out some way for the SSR part to basically run with --location. This will need some more thought.

@FallingSnow
Copy link
Contributor Author

Removing the following seems to be working for the time being.

aleph.js/project.ts

Lines 620 to 633 in 0456c9d

location: {
protocol: 'http:',
host: 'localhost',
hostname: 'localhost',
port: '',
href: 'https://localhost/',
origin: 'https://localhost',
pathname: '/',
search: '',
hash: '',
reload() { },
replace() { },
toString() { return this.href },
},

@shadowtime2000
Copy link
Member

Yeah it prob should work, but I feel like we should look into changing how we do SSR right now. Currently we use a fork of Deno DOM, but as @ije said, we should try avoiding use of deno dom to polyfill everything.

@FallingSnow
Copy link
Contributor Author

Okie.

I do feel if this doesn't break anything and does solve the issue, this or another fix should be added. Would be a bummer if someone came along with everything up to date and aleph fails.

@shadowtime2000
Copy link
Member

I don't know for sure but I think the new compiler is changing the way we do SSR

@ije
Copy link
Member

ije commented Jan 21, 2021

@FallingSnow you are right, i think we can use Object.defineProperty to hack deno's limit.

@ije
Copy link
Member

ije commented Jan 21, 2021

ensure we can get a right location object in SSR, even i don't recommend to access it.

@shadowtime2000
Copy link
Member

@FallingSnow you are right, i think we can use Object.defineProperty to hack deno's limit.

It doesn't seem like that is possible

image

@ije
Copy link
Member

ije commented Jan 21, 2021

ok, i'm going to remove all the browser polyfill objects: 068062f

@shadowtime2000 shadowtime2000 mentioned this issue Jan 21, 2021
15 tasks
@shadowtime2000 shadowtime2000 linked a pull request Jan 21, 2021 that will close this issue
15 tasks
@ije ije closed this as completed in #28 Jan 22, 2021
@ije ije reopened this Feb 1, 2021
@ije
Copy link
Member

ije commented Feb 1, 2021

i just released an alpha version, now it should work in deno 1.7:

deno install -A -f --location=http://localhost -n aleph https://deno.land/x/aleph@v0.3.0-alpha.1/cli.ts

@ije ije closed this as completed Feb 1, 2021
@alundiak
Copy link

alundiak commented May 18, 2021

Just in case, someone will surf for latest 2021 issue... I got deno v1.10.1 and aleph v0.3.0-alpha.33 and initial installation was as:

deno install --unstable -A -f -n aleph https://deno.land/x/aleph@v0.3.0-alpha.33/cli.ts

And then executing aleph dev (project created from aleph init hello_alpha_33) I got:

ERROR invoke API: ReferenceError: Access to "location", run again with --location <href>.
    at get (deno:extensions/web/12_location.js:365:17)
    at createStorage (deno:extensions/webstorage/01_webstorage.js:91:28)
    at localStorage (deno:extensions/webstorage/01_webstorage.js:178:24)
    at handler (file:///...MY../.aleph/development/api/counter/index.js#9786cc:2:28)
    at Server.handle (https://deno.land/x/aleph@v0.3.0-alpha.33/server/server.ts:189:21)

Note: localhost:8080 worked/running well.

I got some reading here and this issue last comment got me a hint, so I re-installed aleph:

deno install -A -f --location=http://localhost -n aleph https://deno.land/x/aleph@v0.3.0-alpha.33/cli.ts

And now aleph dev is "clean".

Not yet sure, if --unstable matters here.

@shadowtime2000
Copy link
Member

@alundiak That is already a problem that has been reported, though an issue has not been made. Could you create one so it could be tracked somewhere else?

@alundiak
Copy link

@shadowtime2000 I wanted to avoid duplication, I've already seen a few similar issues and PRs. So I will not create a new one. Unless it will appear again with deno 1.11.x for example in future.

@shadowtime2000
Copy link
Member

@alundiak I mean like, commenting on a dead issue that was resolved months ago doesn't really help avoid duplication lol. but okay.

@ije
Copy link
Member

ije commented May 20, 2021

the old aleph upgrade command didn't add the location flag that is used by the latest example app in api, i will improve the upgrade command for bootstrap options changes, deno run -A https://deno.land/x/aleph/install.ts can work fine

@ije ije reopened this May 20, 2021
@ije ije closed this as completed May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✘ bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants