[🐛] Require cycle in the lib

[birdofpreyru]

I just tried to migrate existing project from @react-native-firebase/[email protected] to @invertase/[email protected]. Testing it first on Android in dev mode, it seems to fail right away with admob() call returning undefined. RN logs warn about require cycles, ~and that would be my first guess for the problem~: I'll probably debug further and submit a PR later.

P.S.: Glad to see you finally found time to try to make this lib happen, but imho you should be a bit more modest with bumping the major release version on each change. It would make more sense to keep it 0.x.y until it is really tested and confirmed to work in the field.

P.S.S.: Actually, I figured out that in v2.0.0 the default export is just googleMobileAds itself, while according to docs and latest code the function () => googleModileAds is exported.

[DoctorJohn]

Try admob.setRequestConfiguration(...) instead of admob().setRequestConfiguration(...). I had this issue myself experimenting with the current version :)

[birdofpreyru]

Thanks @DoctorJohn , yeah, also noticed it right after creating the bug ticket. Anyway, require cycles should be eliminated, even if not causing particular problems now.

[mikehardy]

All credit to @dylancom for getting it out the door. I won't release 0.x software, that's a semver fail in my opinion. And I will bump major versions any time I hit the release button and it might cause a previously running build to fail. Ask yourself, what does it matter how big a version number is? Quick answer: it does not, the only thing that matters is the signal contained in the number difference. There is no concept of "modesty" in versioning for me, that's a moral concept that doesn't apply to numbers. And so we go...

Sorry it's not working out of the box though - still some growing pains here. Thanks a bunch for joining in with testing.

[mikehardy]

Actually - I think I just released a version that fixes the admob vs admob() fix - check changelog

[mikehardy]

Oh darn - I was waiting for CI to finish and was distracted by children and dinner etc. Hadn't hit the publish button yet. v2.0.1 is going out this moment with the fix that makes the old API work (admob()) - but again all credit to @dylancom - he's fixing the lib here at a furious pace, and it's really appreciated

[birdofpreyru]

I don't know... most projects I saw bump major version only on total API overhaul, introducing landslide breaking changes; and if they think something may break / small breaking changes - they bump minor version, and if some small fixes not requireing any changes in the user's code - then patch version. ¯\_(ツ)_/¯

[mikehardy]

Nope, semantic versioning means one thing and one thing very specifically about the major version changing: it's a breaking change. Anything that breaks running software is a breaking change. It's a really simple definition, and it's fully automated here in our changelog generation and releasing, I literally never touch the versions, I just note what kind of change it is - https://github.com/semantic-release/ - it's really liberating as a philosophy

[zoom2009]

Same here in clean project version "@invertase/react-native-google-ads": "^2.0.1"

import React, { useEffect } from 'react'
import { View } from 'react-native'
import admob, { MaxAdContentRating } from '@invertase/react-native-google-ads'

const App = () => {
  useEffect(() => {

  }, [])

  return (
    <View style={{ flex: 1 }}>
    </View>
  )
}

export default App

[mikehardy]

Yep. Worth noting, require cycles are a warning but may be safely ignored at the moment. They should not be causing any functionality issues that we are aware of. Obviously we want to fix them, but at the moment getting the library running, on current SDKs, using the old APIs (where still possible assuming current SDKs support them) is the highest priority

That said, any PRs that address the require cycles will be appreciated :pray:

[mikehardy]

:tada: This issue has been resolved in version 3.1.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[Darkensses]

Hey @mikehardy I'm seeing this issue again in 6.2.5

[mikehardy]

Well, darn. Thanks for pointing it out @Darkensses - I'll reopen for further triage

[vladimirevstratov]

Stucked with this cycle in react-native-google-mobile-ads v8.2.1

[mikehardy]

Yes, unless you see some other progress, this needs a PR to fix. We'll happily merge and release any reasonable PRs that do so :pray:

[github-actions[bot]]

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.