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

add createJSModules for backward compatibility #354

Merged
merged 1 commit into from Aug 16, 2017

Conversation

@AndrewJack
Copy link
Contributor

AndrewJack commented Aug 9, 2017

#345 removed the method completely. This PR is to provide backward compatibility for older react native versions.

I'm not too bothered which approach we use, remove it or keep for compatibility. Could we have a release soon to allow us to use React Native 0.47?

@isnifer

This comment has been minimized.

Copy link

isnifer commented Aug 10, 2017

@dzhuowen please review, merge and publish to npm

Copy link

isnifer left a comment

Looks good to merge

@philipperocha

This comment has been minimized.

Copy link

philipperocha commented Aug 10, 2017

Please, release soon as possible, cause In .47 has a break changing.

@isnifer

This comment has been minimized.

Copy link

isnifer commented Aug 11, 2017

@dzhuowen where are you?

@RWOverdijk

This comment has been minimized.

Copy link

RWOverdijk commented Aug 12, 2017

👍

@isnifer

This comment has been minimized.

Copy link

isnifer commented Aug 13, 2017

@dzhuowen
image

@AndrewJack

This comment has been minimized.

Copy link
Contributor Author

AndrewJack commented Aug 15, 2017

@punarinta You will not get a compile error in 0.47 if the method isn't annotated with @Override.
It will also compile in 0.46 and older without the @Override annotation. Allowing backward compatibility.

The annotation causes the compile error, it is used as a compile time validation. The compiler will check if the interface has a method of the same name.
See https://docs.oracle.com/javase/tutorial/java/annotations/index.html

This is the same approach as Sentry getsentry/sentry-react-native#172

@mole440

This comment has been minimized.

Copy link

mole440 commented Aug 16, 2017

👍

@sospedra

This comment has been minimized.

Copy link

sospedra commented Aug 16, 2017

👌

@dzhuowen dzhuowen merged commit 39f119c into facebook:master Aug 16, 2017
@dzhuowen

This comment has been minimized.

Copy link
Contributor

dzhuowen commented Aug 16, 2017

Thanks @sospedra for the PR! We'll ship a new version soon.

@farazs

This comment has been minimized.

Copy link

farazs commented Aug 17, 2017

@dzhuowen is there an ETA on that version? The 0.47 changes still haven't been released either so this still doesn't work with RN 0.47+

@joncursi

This comment has been minimized.

Copy link

joncursi commented Aug 21, 2017

Bump!

@rohitgoyal

This comment has been minimized.

Copy link

rohitgoyal commented Aug 21, 2017

Is the version live?
I just tried 0.6.1 but it is still giving me build fail issue because of createjsmodules.

@isnifer

This comment has been minimized.

Copy link

isnifer commented Aug 21, 2017

@rohitgoyal no, 0.6.1 released about 2 weeks ago

@sospedra

This comment has been minimized.

Copy link

sospedra commented Aug 22, 2017

@dzhuowen any news? do we know when it will be ready?

All RN v0.47 projects are blocked because of this ;)
The sooner the better, and the fix is acually merged and ready to fly 🐦

@powdahound

This comment has been minimized.

Copy link

powdahound commented Aug 22, 2017

Hack for people wanting to use this with RN v0.47 before an official release is made: just install the latest commit from GitHub.

$ yarn add git://github.com/facebook/react-native-fbsdk#065507aa1d2b8b0b6cb50d13117694123f8303fa

@ElvisChiang

This comment has been minimized.

Copy link

ElvisChiang commented Aug 23, 2017

patch-package is a better solution. package.json can still track upstream update.

tutorial: easier-react-native-upgrade-with-patch-package

@Alex-Mann

This comment has been minimized.

Copy link

Alex-Mann commented Aug 29, 2017

Any update when this will get merged and released?

@asgvard

This comment has been minimized.

Copy link

asgvard commented Aug 29, 2017

RN 0.48 releasing soon, and there is still no update for 0.47 here. Have to work from fork or specific commit for almost a month already...

@isnifer

This comment has been minimized.

Copy link

isnifer commented Sep 2, 2017

@dzhuowen come on, just do it.

@kidmysoul

This comment has been minimized.

Copy link

kidmysoul commented Sep 7, 2017

Harry Up please. I cannot debug android for nealy 1 month!!

@isnifer

This comment has been minimized.

Copy link

isnifer commented Sep 7, 2017

@kidmysoul install from the github as temporary solution.

"react-native-fbsdk": "facebook/react-native-fbsdk#065507a",
@bodazhao

This comment has been minimized.

Copy link

bodazhao commented Sep 13, 2017

@dzhuowen can there be a quick patch something like 0.6.2?? Every day it's not being fixed there will be a new PR trying to fix it

@RWOverdijk

This comment has been minimized.

Copy link

RWOverdijk commented Sep 14, 2017

How is this still a thing? What's missing?

@asgvard

This comment has been minimized.

Copy link

asgvard commented Sep 14, 2017

@RWOverdijk new npm version of this package is missing :) Everyone just working of a forks or commit refs. Core Facebook RN package takes more than a month to update after breaking change in another Facebook RN package 🎉

@RWOverdijk

This comment has been minimized.

Copy link

RWOverdijk commented Sep 14, 2017

@asgvard I thought there might just be a reason for it. It's fine if people maintain forks, too. It'll probably even lead to an easier to use module because this thing causes so many headaches. I don't suppose facebook will add outside maintainers :p

@kelset

This comment has been minimized.

Copy link

kelset commented Sep 25, 2017

Thanks @isnifer for the snippet, this should be already out, it's not a big lib nor a breaking change ><'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.