nandorojo / dripsy

šŸ· Responsive, unstyled UI primitives for React Native + Web.
https://dripsy.xyz
MIT License
1.98k stars 77 forks source link

Render Error: "Can't find variable key" #265

Closed inakineitor closed 1 year ago

inakineitor commented 1 year ago

Hello!

For some reason, after upgrading from Expo 45, to either 46 or 47, I get the following error on a project running with Dripsy.

image

The code segment the error is referring to is line 61 of node_modules/@dripsy/core/src/css/index.tsx. The syntax appears correct, but for some reason during runtime it is throwing an error as if variable key didn't exist.

code

For the sake of testing I tried updating the error lines to the following and it works perfectly. image image

The pattern seems to point to the error occurring whenever key is not declared inside the block, but inside the for. I have never seen such an error before.

I would really appreciate if anyone was able to shed light on the situation. I've tried for a couple hours to fix this and cannot make it work by installing different Dripsy versions, removing yarn.lock and re-installing. It seems to be tied exclusively to being in an Expo SDK >=46.

Thank you!

P.S.: This also does not work image

nandorojo commented 1 year ago

wow, thatā€™s super odd. it could maybe be a mismatch of expo babel plugins or something. can you run expo doctor?

inakineitor commented 1 year ago

Thank you so much for the prompt response! I ran expo doctor on the repo and it doesn't point to any issues.

Do you know what else it could be?

The packages getting updated in-between versions are:

I don't see what these packages would have to do with Dripsy. I thought TypeScript might have been the culprit (my original upgrade kept the original lower version from my project), so I decided to restart the upgrade and update TypeScript beforehand. That had no effect.

nandorojo commented 1 year ago

Iā€™m thinking itā€™s something related to like the metro bundler, since itā€™s not supporting basic syntax. Or maybe itā€™s no longer transpiling Dripsy or something

nandorojo commented 1 year ago

Did you try expo start -c?

inakineitor commented 1 year ago

I always start after upgrading with yarn ios --clear, which I belive does the same thing since it says "cache empty".

The interesting thing is that is using the .tsx code from Dripsy. I am not updating the .js inside the build folder. To fix it I'm updating the .tsx file.

When I look at the pre-built .js file from before, the for (const key in styles) does not get transpiled since whatever version of ECMAScript that it is transpiling to supports this type of iteration.

What do you think would be the best way to diagnose a problem with the metro bundler?

nandorojo commented 1 year ago

what dripsy version is this? can you try 3.6.0?

Marcin-H commented 1 year ago

I also encountered that error but I'm using "expo": "^45.0.6"

evanrs commented 1 year ago

I encountered this issue as well ā€”Ā but patching Dripsy's package.json to use the build rather than the source fixed the immediate issue.

- "react-native": "src/index.ts"
"react-native": "build/index.js",

I'm unblocked but I'm sure this isn't what @nandorojo has in mind for a solution šŸ˜…

Is there something I can contribute to help diagnose the issue?

Version Details

#### `package.json` ``` "dependencies": "dripsy": "~3.9.0" "expo": "~47.0.3", "react-native": "0.70.5", "devDependencies": "typescript": "^4.9.4" "ts-toolbelt": "^9.6.0", "resolutions": "ts-toolbelt": "^9.6.0", ``` The issue occurred for each version of: - dripsy 3.6 - 3.9 - typescript 4.6 - 4.9

inakineitor commented 1 year ago

Now that we have @evanrs info, we know that the problem is in the interpretation before transpilation.

We can see from the images below that the original lines and the transpiled ones use the native for ... in ... feature, so it is not that the system does not support the feature it is being transpiled to.

image image

Maybe it has to be something with the tsconfig, but after looking at the changelog it appears that that file has not been touched in a while, and that the only thing that changed was the policy on unusedLocalVariables.

@nandorojo I am also free to work on this if you have a direction we should follow. If you want we can implement the first fix (from the beginning of the thread) as a temporal solution. This would make the library work off the bat for new users until we can find what the root issue is.

nandorojo commented 1 year ago

Yeah we can implement that fix. Itā€™s pretty confusing since this is basic syntax to support. I used metro minify esbuild in my metro config, I wonder if thatā€™s why I havenā€™t faced issues?

inakineitor commented 1 year ago

@nandorojo It may be some problem with the actual name key of the variable. On the local copy I have of Dripisy renaming it to anything else, like styleKey, also fixes the issue.

I just made a pull request with that change, but for some reason, it is failing the CI test. It doesn't let me access the Vercel (it appears to be private). Would you be able to tell me what is causing the error in the CI test?

I'll fix it and update the pull request.

nandorojo commented 1 year ago

yeah sure, you can ignore the CI too

inakineitor commented 1 year ago

Oh perfect. Then whenever you have time to review, you can merge the pull request :)

wicharek commented 1 year ago

I have encountered the same issue with my project. It is reproducible with a template expo project (the newest version of expo) + dripsy 3.9.0.

  "dependencies": {
    "dripsy": "^3.9.0",
    "expo": "~47.0.9",
    "expo-status-bar": "~1.4.2",
    "react": "18.1.0",
    "react-dom": "18.1.0",
    "react-native": "0.70.5",
    "react-native-web": "~0.18.9"
  },
  "devDependencies": {
    "@babel/core": "^7.12.9",
    "@types/react": "~18.0.14",
    "@types/react-native": "~0.70.6",
    "prettier": "^2.8.1",
    "typescript": "^4.6.3"
  },

Here is a minimal, reproducible example project: https://github.com/wicharek/dripsy-key-crash

Hope that helps.

jcamden commented 1 year ago

Just want to confirm that manually implementing the fix from @inakineitor solved this for me until that gets merged. Thanks a bunch!

bombillazo commented 1 year ago

I hit this error recently too, and renaming key fixed it for us as well.

nandorojo commented 1 year ago

Iā€™ll release the fix soon, just merged the PR

nandorojo commented 1 year ago

Released in 3.10

jeremyfrancis commented 1 year ago

@nandorojo It may be some problem with the actual name key of the variable. On the local copy I have of Dripisy renaming it to anything else, like styleKey, also fixes the issue.

I just made a pull request with that change, but for some reason, it is failing the CI test. It doesn't let me access the Vercel (it appears to be private). Would you be able to tell me what is causing the error in the CI test?

I'll fix it and update the pull request.

Could you provide more specific direction on how to achieve what you achieved? Like the file name... ?

inakineitor commented 1 year ago

Hello!

For some reason, after upgrading from Expo 45, to either 46 or 47, I get the following error on a project running with Dripsy.

image

The code segment the error is referring to is line 61 of node_modules/@dripsy/core/src/css/index.tsx. The syntax appears correct, but for some reason during runtime it is throwing an error as if variable key didn't exist.

code

For the sake of testing I tried updating the error lines to the following and it works perfectly. image image

The pattern seems to point to the error occurring whenever key is not declared inside the block, but inside the for. I have never seen such an error before.

I would really appreciate if anyone was able to shed light on the situation. I've tried for a couple hours to fix this and cannot make it work by installing different Dripsy versions, removing yarn.lock and re-installing. It seems to be tied exclusively to being in an Expo SDK >=46.

Thank you!

P.S.: This also does not work image

It's the function called responsive on line file node_modules/@dripsy/core/src/css/index.tsx (second picture above). I don't believe it happens in any other file (or at least I have not encountered that error). @nandorojo pulled the changes to the main branch so if you upgrade the npm package the issue should fix itself without needing to modify the source code yourself.

This approach is recommended so that you don't have to patch the code every time npm decides to refresh the node_modules content.

Lmk if it works!

jeremyfrancis commented 1 year ago

@inakineitor thank you so much! I updated to 3.10.0 and just needed to expo run:ios again and it started running properly.