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

Not working with CRNA #940

Closed
deadcoder0904 opened this issue Nov 17, 2017 · 23 comments
Closed

Not working with CRNA #940

deadcoder0904 opened this issue Nov 17, 2017 · 23 comments

Comments

@deadcoder0904
Copy link
Contributor

I tried it in the previous release too & also in this release but the only reason I can't use PNPM is because it doesn't work using CRNA.

Now its giving error - Cannot find module 'babel-plugin-transform-flow-strip-types'

Steps to repro -

create-react-native-app myApp
rm -rf node_modules && rm yarn.lock
pnpm i
pnpm start
@deadcoder0904
Copy link
Contributor Author

Also, React Native CLI react-native init. Throws some weird error mentioned above.

@jaystarshot
Copy link

Hey I am new to open Source ,can I help in any way?

@zkochan
Copy link
Member

zkochan commented Jan 11, 2018

In this particular case probably the cause of the error should be identified. Why babel-plugin-transform-flow-strip-types cannot be found. Maybe it is not declared in package.json of the dependency that requires it.

In general, if you want to help, you can start with issues marked as easy, help wanted, good first issue. Or just read the titles... there's not that match

@deadcoder0904
Copy link
Contributor Author

Naah. Same thing works with npm & yarn. That's the reason the scope is so narrow. I couldn't understand the problem.

@zkochan
Copy link
Member

zkochan commented Jan 11, 2018

Both npm and yarn create a flat node_modules structure and the package can access many dependencies that are not declared in package.json. I'd say in 95% of cases (and the value is getting closer to 100% as we fix more and more issues) the issues are that packages require dependencies that are not declared in package.json.

@zkochan
Copy link
Member

zkochan commented Jan 11, 2018

oh, or the other issue might be this: #801

some of the most popular resolvers don't adhere Node's resolution algorithms. They preserve symlinks when resolving. pnpm creates lots of symlinks and it works with Node because it does not preserve them

@zkochan
Copy link
Member

zkochan commented Jan 11, 2018

@deadcoder0904 I just checked. It is an issue with metro-bundler. It uses a bunch of packages that are not declared in its package.json. They should fix it.

With a pnpm hook, I added the missing dependencies to its manifest and it worked.

pnpmfile.js:

'use strict'
module.exports = {
  hooks: {
    readPackage
  }
}

function readPackage (pkg) {
  if (pkg.name === 'metro-bundler') {
    Object.assign(pkg.dependencies, {
      'babel-plugin-transform-flow-strip-types': '6',
      'babel-plugin-transform-object-rest-spread': '6',
      'babel-plugin-transform-class-properties': '6',
      'babel-plugin-transform-async-to-generator': '6',
      'babel-plugin-syntax-trailing-function-commas': '6',
      'xtend': '*',
      'errno': '*',
    })
  }
  return pkg
}

@deadcoder0904
Copy link
Contributor Author

Well thanks @zkochan. I'll file an issue there.

@deadcoder0904
Copy link
Contributor Author

@zkochan How did you find the missing dependencies ? Do you already have any available script or should I just check the source & anyone requireing or importing packages not available in package.json should be added to the hook?

This might help me with #991

@zkochan
Copy link
Member

zkochan commented Jan 28, 2018

In most cases, there is a Cannot find module error with a stacktrace. The stacktrace points to the package that is missing the dependency

@deadcoder0904
Copy link
Contributor Author

Yep got it

This was referenced Jan 28, 2018
@vjpr
Copy link
Contributor

vjpr commented Mar 1, 2018

Updated pnpmfile.js that works for me.

'use strict'
module.exports = {
  hooks: {
    readPackage,
  },
}

function readPackage(pkg) {
  if (pkg.name === 'metro-core') {
    Object.assign(pkg.dependencies, {
      wordwrap: '*',
    })
  }
  if (pkg.name === 'metro') {
    Object.assign(pkg.dependencies, {
      'babel-plugin-transform-flow-strip-types': '6',
      'babel-plugin-transform-object-rest-spread': '6',
      'babel-plugin-transform-class-properties': '6',
      'babel-plugin-transform-async-to-generator': '6',
      'babel-plugin-syntax-trailing-function-commas': '6',
      xtend: '*',
      errno: '*',
    })
  }
  return pkg
}

I still couldn't get it to work. There are some more issues related to react-native not supporting symlinks. See facebook/react-native#18156

@vjpr
Copy link
Contributor

vjpr commented Mar 13, 2018

So I got react-native working with pnpm. #1051 (comment)

I will take a look at getting it working for CRNA soon.

@deadcoder0904
Copy link
Contributor Author

Thanks man, appreciate it

@vjpr
Copy link
Contributor

vjpr commented Mar 15, 2018

Got CRNA working with pnpmfile.js. Was a bit tricky because expo hides errors from the react-native packager and it uses an older version of react-native which I had to create a custom patch of metro.

Will tidy this up later.

'use strict'
module.exports = {
  hooks: {
    readPackage,
  },
}

// NOTE: To see crna packager errors you must add `console.log(JSON.stringify(msg, null, 2))` to `this._handlePackagerEvent` here:
//   photo-booth-ios-crna/node_modules/.registry.npmjs.org/xdl/48.0.2/node_modules/xdl/build/logs/PackagerLogsStream.js

// NOTE: Test changes with `pnpm up`.
// NOTE: For `console.log` to appear correctly use: `pnpm up --reporter=append-only`.

function readPackage(pkg) {
  require('pnpmfile-check').default(pkg)

  // `metro` pnpm support.
  // TODO(vjpr): Use semver instead to test metro version.
  if (pkg.name === 'react-native' && pkg.version === '0.52.0') {
    Object.assign(pkg.dependencies, {
      'metro': 'npm:metro-pnpm@0.24.7-vjpr.1',
      //'metro-resolver': '~/dev-live/metro/.dev/metro-resolver-0.28.0.tgz',
      'uuid': '*', // Libraries/Blob/Blob.js
    })
  }
  if (pkg.name === 'react-native-scripts') {
    Object.assign(pkg.dependencies, {
      expo: '*',
    })
  }

  if (pkg.name === 'metro-core') {
    console.log('adding wordwrap to metro-core')
    Object.assign(pkg.dependencies, {
      wordwrap: '*',
    })
  }
  // expo/src/Notifications.js
  if (pkg.name === 'expo') {
    Object.assign(pkg.dependencies, {
      fbjs: '^0.8.16',
      lodash: '*',
      'json-schema-traverse': '*',
    })
  }

  // 1.0.0-alpha.39
  if (pkg.name === 'react-native-gesture-handler') {
    Object.assign(pkg.dependencies, {
      fbjs: '^0.8.16',
    })
  }

  // @expo/vector-icons@6.3.1
  if (pkg.name === '@expo/vector-icons') {
    Object.assign(pkg.dependencies, {
      'prop-types': '*',
    })
  }

  if (pkg.name === 'metro-pnpm') {
    Object.assign(pkg.dependencies, {
      'babel-plugin-transform-flow-strip-types': '6',
      'babel-plugin-transform-object-rest-spread': '6',
      'babel-plugin-transform-class-properties': '6',
      'babel-plugin-transform-async-to-generator': '6',
      'babel-plugin-syntax-trailing-function-commas': '6',
      'babel-template': '6',
      resolve: '*',
      xtend: '*',
      errno: '*',
    })
  }

  /*
  pnpm ls babel-preset-expo

  ├─┬ expo@25.0.0
  │ └── babel-preset-expo@4.0.0
  └─┬ react-native-scripts@1.11.1
    └─┬ expo@25.0.0
      └── babel-preset-expo@4.0.0
  */
  const pjson = require('./package.json')
  if (pkg.name === pjson.name) {
    Object.assign(pkg.dependencies, {
      'babel-preset-expo': '^4',
      'babel-plugin-transform-react-jsx-source': '6',
    })
  }
  return pkg
}

@deadcoder0904
Copy link
Contributor Author

@vjpr Doesn't --shamefully-flatten help with this❓

@vjpr
Copy link
Contributor

vjpr commented Mar 15, 2018

@deadcoder0904 --shamefully-flatten would help with the missing deps, yes, but this is easy and safer to fix in the pnpmfile.

But the main challenge was patching metro. And patching it in multiple versions because Expo uses an older version of react-native.

@vjpr
Copy link
Contributor

vjpr commented Mar 16, 2018

Here is a more automated way to work with CRNA.

I will be removing the babel-register dependency tomorrow.

pnpmfile.js

require('babel-register')
module.exports = require('./pnpmfile/index')

pnpmfile/package.json

{
  "name": "pnpmfile",
  "version": "0.0.1",
  "devDependencies": {
    "@pnpmfile/create-react-native-app": "^0.0.2-0"
  },
  "dependencies": {
    "babel-preset-env": "^1.6.1"
  }
}

pnpmfile/index.js

export default {
  hooks: {
    readPackage,
  },
}

function readPackage(pkg) {
  //require('pnpmfile-check').default(pkg)
  console.log({pkg})
  require('@pnpmfile/create-react-native-app').default(pkg)
  if (pkg.name === 'photo-booth-ios-crna') {
    console.log(pkg.dependencies)
  }
  return pkg
}

package.json

{ 
  dependencies: {
    'pnpmfile': '*',
    'babel-register: '^6'
  }
}

You must run the following to ensure that pnpmfile deps are installed first.

pnpm recursive install

@vjpr vjpr closed this as completed Mar 16, 2018
@HarshulSharma000
Copy link

@vjpr In short now all the upcoming release are supposed to work correctly. I hope this fixes the wordwrap not found and other babel missing package errors also.

@vjpr
Copy link
Contributor

vjpr commented Mar 18, 2018

@HarshulSharma000

In short now all the upcoming release are supposed to work correctly.

Nope. I can only guarantee this works for react-native@<=0.54.2, because I manually patch a react-native dep.

There was a re-org done from 0.52 -> 0.54, but the functions I patched did not change.

Until a PR in react-native is merged each new version will need a new patch. I will try do this soon.

For CRNA + EXPO versioning see: https://github.com/react-community/create-react-native-app/blob/master/VERSIONS.md


I hope this fixes the wordwrap not found and other babel missing package errors also.

This is fixed, and will not be problematic between versions.


Also, patches will take me <5mins for each new version because I have automated it. So unless something big changes in react-native then it should be easy to keep it up to date.

I am maintaining the pnpmfile hook here: https://github.com/vjpr/pnpmfile/blob/master/packages/create-react-native-app/def.js

@bkniffler
Copy link

Is the package still maintained @vjpr ? I've not been able to get things working, getting expo/CRNA to work with pnpm ist a pain (I know this is mostly due to path resolving of expo).

@vjpr
Copy link
Contributor

vjpr commented Nov 18, 2018 via email

@vjpr
Copy link
Contributor

vjpr commented Nov 18, 2018

Part 1 - Getting expo-cli working with pnpm: expo/expo-cli#183

Will track progress here: #1501

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

No branches or pull requests

6 participants