[email protected] update

[idkjs]

I have updated the package to [email protected]. There maybe a few places where we could remove the . notations where js objects are inlined, example in AR.re https://github.com/idkjs/reason-expo/blob/0e7a1f87eb6adbcbeb216b52667b8105179558dd/packages/reason-expo/src/AR.re#L522-L566

Not sure if the tests need to be adjusted. As of now, they all pass as is.

I have add new usage comments at the top of AuthSession.re. The old ones will have to be removed once accepted. https://github.com/idkjs/reason-expo/blob/e492bd72c98b7c4ca3bbc15a99d8da716d531359/packages/reason-expo/src/AuthSession.re#L14-L22

I would also like some feedback on AdMob.re and whether to use Js.Nullable.return or Js.Nullabe.fromOption in the AdMob.AdMobBanner module.

While these lines are compiling and passing tests, I dont trust them: https://github.com/idkjs/reason-expo/blob/e492bd72c98b7c4ca3bbc15a99d8da716d531359/packages/reason-expo/src/AdMob.re#L41-L58

Looking forward you your feedback to see how to proceed.

Peace/Love.

[peterpme]

Hey @idkjs so sorry for the delay! Is this still good to go?

[idkjs]

Hi @peterpme. I would have to go back over it since we are up to [email protected] and there were some breaking changes for some of the syntax that we came across in other repos. I will go ahead and do that. Yes?

[peterpme]

That would be great, thank you so much @idkjs !

[idkjs]

@peterpme this last commit updates the deps to the following:

"dependencies": {
    "expo": "~36.0.0",
    "react": "~16.9.0",
    "react-dom": "~16.9.0",
    "react-native": "https://github.com/expo/react-native/archive/sdk-36.0.0.tar.gz",
    "reason-expo": "^36.0.0",
    "react-native-web": "~0.11.7",
    "reason-react-native": "^0.61.1",
    "reason-react": "^0.7.0"
  },
  "devDependencies": {
    "@babel/core": "^7.0.0",
    "babel-preset-expo": "~8.0.0",
    "bs-platform": "^7.2.2",
    "expo-yarn-workspaces": "^1.2.1"
  },

In packages/test rn-cli.config.js becomes metro.config.js per expo expo-yarn-workspaces.

I moved around the types in ImageManipulator.re and Camera.re to get rid of the bucklescript warnings.

.gitAttributes updates OCaml to Reason.

I feel like there are too many deps in https://github.com/idkjs/reason-expo/blob/cf55209ba848248702446c82fabd4b2bc24bcec6/packages/reason-expo/package.json#L24-L41 based on what was there before but that is what I was left with after working through the build errors. Maybe you have some insight there.

Using sdk-36.0.0.

The demo runs on web:

And Ios:

Not sure if that is what you all wanted to do. Thoughts?

[peterpme]

so far looks great! just a couple merge conflicts

[idkjs]

So work with me, brother. What is the protocol for resolving merge conflicts? Is that for me to do? if so how do I see the conflict? I'm greyed out in this ui.

[peterpme]

Hey @idkjs, I actually just noticed that as well! After some investigation, I'm not sure why .gitattributes is a conflict because it doesn't exist.

Is it easy for you to pull down master and then merge it into your branch?

For both .gitattributes and packages/reason-expo/package.json override your new version. I looked into how Expo's new stuff works and it aligns with what you're doing.

That should make fixing the merge conflict easy!

[idkjs]

It would be easy if I new how to do it. Seems like if I do that, it will override all everything I updated, no? I could just change the .gitattributes manually but not sure what the problem is with package.json. Any guidance? Thanks.

[peterpme]

I don't think so:

on your local fork:

git checkout master
git pull
git checkout idkjs:bs_platform_7
git merge master

it should be as that? maybe i'm missing something?

[idkjs]

Ran your commands:

~/G/reason-expo ❯❯❯ git branch
* bs_platform_7
  master
~/G/reason-expo ❯❯❯ git checkout master
Already on 'master'
Your branch is up to date with 'origin/master'.
~/G/reason-expo ❯❯❯ git pull
Already up to date.
~/G/reason-expo ❯❯❯ git checkout bs_platform_7
Switched to branch 'bs_platform_7'
Your branch is up to date with 'origin/bs_platform_7'.
~/G/reason-expo ❯❯❯ git merge master
Auto-merging packages/reason-expo/src/SQLite.re
Auto-merging packages/reason-expo/package.json
CONFLICT (content): Merge conflict in packages/reason-expo/package.json
CONFLICT (modify/delete): .gitattributes deleted in master and modified in HEAD. Version HEAD of .gitattributes left in tree.
Automatic merge failed; fix conflicts and then commit the result.
~/G/reason-expo ❯❯❯                                                                                                         ✘ 1 

So how do we fix these conflicts?

[peterpme]

Hey @idkjs nice! Really close. One merge conflict in package.json. Then a few questions pertaining to the Svg.re file and .gitattributes that was deleted

[idkjs]

I think we are good now. Other issues?

Thank you.

[zth]

Nice work @idkjs , looking forward to using this once it lands!

[sagarjs]

any updates on this pull request? looking forward to the merge

[peterpme]

Hi sorry about this delay, thank you @idkjs !!!