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

Failed to import - @reduxjs/toolkit #681

Closed
etodanik opened this issue Jul 12, 2023 · 10 comments
Closed

Failed to import - @reduxjs/toolkit #681

etodanik opened this issue Jul 12, 2023 · 10 comments
Labels
deno Not working in Deno

Comments

@etodanik
Copy link

etodanik commented Jul 12, 2023

Failing module

import { composeWithDevTools} from "@reduxjs/toolkit"

Error message

After running deno run I got this:
ReferenceError: window is not defined

It seems like it's because of the following line here:
https://esm.sh/v97/@reduxjs/toolkit@1.9.5/denonext/toolkit.development.mjs:180:60

var composeWithDevTools = typeof Deno   !== "undefined" && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ ? window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ : function() {

It's checking for Deno to not be undefined, but then immediately accessing window, hence triggering the ReferenceError

Additional info

  • esm.sh version: v128
  • Deno version: 1.35.0
@etodanik etodanik added the deno Not working in Deno label Jul 12, 2023
@dsosby
Copy link

dsosby commented Jul 15, 2023

I think it's from this recent change here

It seems to string replace typeof window when the target is Deno, which may be set due to the Deno HttpAgent user-agent.

A similar issue in the Deno Fresh framework here

@marvinhagemeister
Copy link
Contributor

marvinhagemeister commented Jul 15, 2023

Good catch! I was debugging esbuild itself and didn't consider that this error would be introduced by esm.sh . Replacing window checks with Deno seems very brittle as most frontend libraries use it to determine if they are running in the browser.

@ije
Copy link
Member

ije commented Jul 15, 2023

ideally you should use target es20xx for browsers, however i will get rid of the replacing for deno target

@ije ije closed this as completed in 7d63a99 Jul 15, 2023
@marvinhagemeister
Copy link
Contributor

@ije The issue is still present, see https://esm.sh/stable/preact@10.15.1/denonext/devtools.mjs where typeof window is replaced with typeof Deno

@ije
Copy link
Member

ije commented Jul 17, 2023

@marvinhagemeister not deploy the patch yet, will do it tmr and let you know, thanks

@deer
Copy link

deer commented Aug 6, 2023

This doesn't seem to be a "one size fits all" type of fix.

In https://esm.sh/v128/@reduxjs/toolkit@1.9.5/denonext/toolkit.development.mjs the problematic line has changed from:

var composeWithDevTools = typeof Deno   !== "undefined" && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ ?

to

var composeWithDevTools = typeof window !== "undefined" && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__

in https://esm.sh/v129/@reduxjs/toolkit@1.9.5/denonext/toolkit.development.mjs. (Note the version numbers in the URLs are different.) So the fix deployed in v129 (7d63a99) which resolved this definitely did work. Great.

But there was hope from the fresh community that this would also fix the preact debugging issue. But both of the following:

have this code:

typeof Deno  <"u"&&window.__PREACT_DEVTOOLS__

Should I log a separate issue for this?

@ije
Copy link
Member

ije commented Aug 6, 2023

@deer can you please ask the fresh community update to v129+?

@deer
Copy link

deer commented Aug 6, 2023

Sorry, I think I wasn't clear with my comment. I don't think this has anything in particular to do with fresh. I'll try to explain better a second time:

The preact source has a line like this:

if (typeof window != 'undefined' && window.__PREACT_DEVTOOLS__) {

and both esm.sh v128 and esm.sh v129 produce something like this:

typeof Deno  <"u"&&window.__PREACT_DEVTOOLS__

Contrast this with the redux toolkit source which has a line like:

typeof window !== 'undefined' &&
  (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__

esm.sh v128 has it like:

typeof Deno   !== "undefined" && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__

but esm.sh v129 has it like:

typeof window !== "undefined" && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__

The hope was that this would get resolved for the preact devtools, but it seems like we're still getting typeof Deno, instead of the desired typeof window (which redux now gets). Am I misunderstanding something? I don't see how doing anything with fresh would change the results of these URLs.

@dsosby
Copy link

dsosby commented Aug 6, 2023

https://esm.sh/v129/preact@10.15.1/denonext/devtools.mjs

I notice that adding ?dev yields working code for this particular package (that is, has typeof window instead of typeof Deno)

@dsosby
Copy link

dsosby commented Aug 6, 2023

The preact devtools package seems to be a different bug(?). Should it be a different issue? The prior behavior for reduxjs/toolkit could be bypassed by giving an explicit target, but that does not work in this case:

https://esm.sh/v129/preact@10.15.1/denonext/devtools.mjs?target=es2022 still has typeof Deno

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno Not working in Deno
Projects
None yet
Development

No branches or pull requests

5 participants