uuidjs / uuid

Generate RFC-compliant UUIDs in JavaScript
MIT License
14.54k stars 896 forks source link

Not Compatible with React Native #408

Closed judygab closed 4 years ago

judygab commented 4 years ago

Describe the bug

I installed uuid in my react native project, following the instructions in readme and encountered problems with it. I followed the steps of clearing caches, deleting node modules and installing. I'm attaching the image of the error that I am getting.

Screen Shot 2020-03-24 at 9 47 51 AM

Runtime

LinusU commented 4 years ago

In version 7 of UUID we have deprecated the direct imports. Instead of importing from uuid/v1, could you try to import it as such:

import { v1 as uuidv1 } from 'uuid'

uuidv1() // ⇨ '2c5ea4c0-4067-11e9-8b2d-1b9d6bcdbbfd'
ctavan commented 4 years ago

As commented by @LinusU, deep requires are deprecated. It was indeed unintended that we actually broke them for non-node environments, but we will probably not change that.

See https://github.com/uuidjs/uuid#deep-requires-now-deprecated for more information.

judygab commented 4 years ago

This breaks before the import though.

LinusU commented 4 years ago

It probably breaks when constructing the dependency tree, but if you change the import that should fix it. Did you try to change the import?

ctavan commented 4 years ago

Hmm, we could use https://github.com/TooTallNate/util-deprecate 🤔

judygab commented 4 years ago

Yeah, I tried that import but still getting the same error, does this require installing pods?

ctavan commented 4 years ago

@judygab did follow the polyfilling instructions from https://github.com/LinusU/react-native-get-random-values#readme ?

What does your exact import code look like?

kopax commented 4 years ago

Hello, I just discovered your module and I am using react-native.

My target are iOS, Android and web, this is my import:

import { v4 as uuidv4 } from 'uuid';

It work fine with iOS and Android, but I have the following new error on the web:

Deep requiring like `const uuidv4 = require('uuid/v4');` is deprecated as of uuid@7.x. Please require the top-level module when using the Node.js CommonJS module or use ECMAScript Modules when bundling for the browser. See https://github.com/uuidjs/uuid#deep-requires-now-deprecated for more information.

There's obviously something wrong with uuidjs.

I am using uuid v7.0.2, any idea how I can fix this?

ctavan commented 4 years ago

@kopax Can you clear all build caches/node_modules directories/etc. and try again? This looks like some legacy code survived the upgrade and not like a problem with the uuid library.

In particular, search your code base for uuid/v4 and upgrade all legacy import statements as described in https://github.com/uuidjs/uuid#deep-requires-now-deprecated

kopax commented 4 years ago

@ctavan your assertion is wrong, I have never used your module before. I never heard of it before today when I was looking for a solution on how to get a filename for pictures taken with the webcam/camera in expo SDK36.

I did npm ls uuid and this is what I got:

test-native@1.1.0 /home/dka/workspace/test-native
├─┬ @expo/webpack-config@0.11.3
│ └─┬ copy-webpack-plugin@5.0.0
│   └─┬ webpack-log@2.0.0
│     └── uuid@3.4.0 
├─┬ @rollup-umd/documentation@2.1.1
│ ├─┬ image-webpack-loader@6.0.0
│ │ └─┬ imagemin-gifsicle@6.0.1
│ │   └─┬ exec-buffer@3.2.0
│ │     └─┬ tempfile@2.0.0
│ │       └── uuid@3.4.0 
│ └─┬ react-styleguidist@9.1.2
│   └─┬ webpack-dev-server@3.10.3
│     └─┬ sockjs@0.3.19
│       └── uuid@3.4.0 
├─┬ @semantic-release/npm@7.0.4
│ └─┬ npm@6.14.2
│   ├─┬ request@2.88.0
│   │ └── uuid@3.3.3  deduped
│   └── uuid@3.3.3 
├─┬ @yeutech-lab/expo-cli@3.12.1-fix-1405.4
│ ├─┬ @expo/build-tools@0.1.3
│ │ └── uuid@3.4.0 
│ ├─┬ @expo/bunyan@3.0.2
│ │ └── uuid@3.4.0 
│ ├─┬ @expo/dev-tools@0.10.1
│ │ └─┬ graphql-tools@3.0.0
│ │   └── uuid@3.4.0 
│ ├─┬ @expo/xdl@57.5.1
│ │ ├─┬ @expo/ngrok@2.4.3
│ │ │ └── uuid@3.4.0 
│ │ ├─┬ analytics-node@3.3.0
│ │ │ └── uuid@3.4.0 
│ │ ├─┬ raven@2.6.3
│ │ │ └── uuid@3.0.0 
│ │ ├─┬ slugid@1.1.0
│ │ │ └── uuid@2.0.3 
│ │ ├── uuid@3.3.2 
│ │ └─┬ xcode@2.1.0
│ │   └── uuid@3.4.0 
│ ├─┬ match-require@2.1.0
│ │ └── uuid@3.4.0 
│ └─┬ request@2.88.0
│   └── uuid@3.4.0 
├─┬ expo-cli@3.13.5
│ ├─┬ @expo/build-tools@0.1.4
│ │ └── uuid@3.4.0 
│ └─┬ @expo/xdl@57.5.4
│   └── uuid@3.3.2 
├─┬ jsdom@16.2.1
│ └─┬ request@2.88.2
│   └── uuid@3.4.0 
└── uuid@7.0.2 

Looking at my node_modules/uuid/package.json, this is the content:

"_id": "uuid@7.0.2",
TrySound commented 4 years ago

Some module may not list its uuid in dependencies and use the root one instead. This might trigger warning. Try to add console.trace in uuid/v4.js to see which module trigger this.

kopax commented 4 years ago

../../../uuid/v4.js | @ | bundle.js:formatted:251948
-- | -- | --
  | __webpack_require__ | @ | bundle.js:formatted:1460
  | fn | @ | bundle.js:formatted:253
  | ../../../expo-constants/build/ExponentConstants.web.js | @ | bundle.js:formatted:96205
  | __webpack_require__ | @ | bundle.js:formatted:1460
  | fn | @ | bundle.js:formatted:253
  | ../../../expo-constants/build/Constants.js

You're assertion is correct, it is happening here : https://github.com/expo/expo/blob/master/packages/expo-constants/src/ExponentConstants.web.ts#L2

Any idea how I can fix this? am I subject to breaking expo by leaving this? I believe not, it's just a warning for the import, it would be nice to be able to downgrade the warning so we can perform the upgrade without adding more warning to our apps.

I am noww choosing to dowgrade my uuid version to v3.4.0 to remove those warnings.

TrySound commented 4 years ago

Looks like they fixed this in master. Need to ping them to publish new version.

kopax commented 4 years ago

I have checked the commit and the PR title is a bit confusing... How did they "make it work"? Did they just removed it?

TrySound commented 4 years ago

https://github.com/expo/expo/commit/d0bc483df71b999d539f16e94f276fba70e2c65a#diff-ce3eb7300b89a57b05be12979132e0bcR43-R46

kopax commented 4 years ago

Thanks for the link.

Need to ping them to publish new version.

Expo is not publishing often so I'll just stay with v3.4.0 till they upgrade. I believe this will still do the job.

judygab commented 4 years ago

In version 7 of UUID we have deprecated the direct imports. Instead of importing from uuid/v1, could you try to import it as such:

import { v1 as uuidv1 } from 'uuid'

uuidv1() // ⇨ '2c5ea4c0-4067-11e9-8b2d-1b9d6bcdbbfd'

I have tried that, didn't work.

ctavan commented 4 years ago

@judygab maybe you're having the same issue as https://github.com/uuidjs/uuid/issues/408#issuecomment-606070136 ?

judygab commented 4 years ago

@ctavan I don't think so, I'm not using Expo.

TrySound commented 4 years ago

Any package may forget to add uuid to dependencies. Try to add console.trace too.

judygab commented 4 years ago

Agreed, but this was not happening until I installed uuid specifically.

ctavan commented 4 years ago

@judygab I think that we're not finding a solution to your problem with further guesswork.

Could you please provide exact steps to reproduce (including all source code necessary to run into the error) so that we can find your issue (http://sscce.org/)? Thanks!

juhana commented 4 years ago

The same error happens on a clean React Native project with no other dependencies added except uuid. If it's caused by an old version being used in the project, it looks like React Native's xcode package uses version 3.4.0 (and Jest, but that should affect only tests.)

➜ yarn why uuid
yarn why v1.22.4
[1/4] 🤔  Why do we have the module "uuid"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "uuid@8.0.0"
info Has been hoisted to "uuid"
info This module exists because it's specified in "dependencies".
info Disk size without dependencies: "240KB"
info Disk size with unique dependencies: "240KB"
info Disk size with transitive dependencies: "240KB"
info Number of shared dependencies: 0
=> Found "xcode#uuid@3.4.0"
info This module exists because "react-native#@react-native-community#cli-platform-ios#xcode" depends on it.
info Disk size without dependencies: "92KB"
info Disk size with unique dependencies: "92KB"
info Disk size with transitive dependencies: "92KB"
info Number of shared dependencies: 0
=> Found "node-notifier#uuid@7.0.3"
info This module exists because "jest#@jest#core#@jest#reporters#node-notifier" depends on it.
info Disk size without dependencies: "252KB"
info Disk size with unique dependencies: "252KB"
info Disk size with transitive dependencies: "252KB"
info Number of shared dependencies: 0
=> Found "request#uuid@3.4.0"
info This module exists because "jest#@jest#core#jest-config#jest-environment-jsdom#jsdom#request" depends on it.
info Disk size without dependencies: "92KB"
info Disk size with unique dependencies: "92KB"
info Disk size with transitive dependencies: "92KB"
info Number of shared dependencies: 0
✨  Done in 0.88s.
ctavan commented 4 years ago

@judygab are you still experiencing this issue?

judygab commented 4 years ago

@judygab are you still experiencing this issue?

Yes, I could not find the solution and ended up not using the module.

ctavan commented 4 years ago

Sad to hear this @judygab.

@juhana were you able to solve your issue?

juhana commented 4 years ago

When I tried it last week, it didn't work because of other problems with React Native that are tracked in other issues. But as far as I can tell this specific issue is not a problem anymore.

ctavan commented 4 years ago

Thanks for the update @juhana !

Closing since it doesn't seem to affect active users anymore. Please feel free to open a new issue if anything comes up.