alephjs / aleph.js

The Full-stack Framework in Deno.
https://aleph.deno.dev/
MIT License
5.26k stars 168 forks source link

Deno 1.7.0 incompatibility #92

Closed FallingSnow closed 3 years ago

FallingSnow commented 3 years ago
$ 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 https://github.com/denoland/deno/issues/4981#issuecomment-685205034

FallingSnow commented 3 years ago

See for more info. https://github.com/alephjs/aleph.js/blob/0456c9da64ba09967a1b376358c93e8991bdff97/project.ts#L620

FallingSnow commented 3 years ago

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

shadowtime2000 commented 3 years ago

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

FallingSnow commented 3 years ago

Removing the following seems to be working for the time being. https://github.com/alephjs/aleph.js/blob/0456c9da64ba09967a1b376358c93e8991bdff97/project.ts#L620-L633

shadowtime2000 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

ije commented 3 years ago

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

ije commented 3 years ago

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

shadowtime2000 commented 3 years ago

@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 commented 3 years ago

ok, i'm going to remove all the browser polyfill objects: https://github.com/alephjs/aleph.js/pull/28/commits/068062fb94e9b08fafd6d23e42b35ab9d317b0a3

ije commented 3 years ago

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
alundiak commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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

ije commented 3 years ago

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