feat: scraping expo/react-native version from package.json

[giautm]

📝 Why & how

Determining a compatible react-native version

Guess what react-native version is supported based on dependencies should be for reference only as we have no other information. In the future, we may describe a special field to report that. Or, we can also use the information from the rnx-kit field to know the supported version.

package.json:

{
  "name": "example-a",
  "version": "0.2.0",
  "@react-native-community/directory": {
    "expoSdkVersion": "45.0.0",
    "reactNativeVersion": "0.58.0"
  }
}

The steps will be: check the @react-native-community/directory field -> check the rnx-kit field -> check dependencies.

However, I still think we should have a database, where people can announce the version and their support, and update automatically every time a new version is released.

✅ Checklist

• Documented in this PR how to use the feature or replicate the bug.
• Documented in this PR how you fixed or created the feature.

[Simek]

What's the reason for storing new data in separate files per version, instead of populating library entry in data.json?

Also, what purpose will serve added expo-react-native.json file?

[giautm]

What's the reason for storing new data in separate files per version, instead of populating library entry in data.json?

I have explain about this approach in Evan's notion doc. It's allow us pinned correct version of each library for the RN version.

Also, what purpose will serve added expo-react-native.json file?

This is a config file, to say: what are RN and Expo versions we support to looking up.

[Simek]

I have explain about this approach in Evan's notion doc.

I see, this have been added after my first read of the doc. 🙂

It's allow us pinned correct version of each library for the RN version.

Yeah, but I still doesn't see the point of creating separate files, instead of populating entry in data.json. For example:

{
  "name": "expo-av",
  ...,
  "rnVersions": {
    "0.64.0" : "0.1.0"
  },
  "expoVersions": [
    {
      expo: "45.0.0",
      reactNative: "0.68",
      packageVersion: "0.2.1",
      added: TS,
      ...
    },
    {
      expo: "44.0.0",
      reactNative: "0.64",
      packageVersion: "0.1.7",
      added: TS,
      ...
    },
    ...
  ]
}

Both of those fields can also be an array to include object per version, so we can include even more information if needed.

Also the older npm releases are always available, so we can fetch few previous versions data per package to get the information for older versions of packages, which removes the need for storing any other JSON files IMO.

This also allow us to utilize the new version data in the directory itself without many changes, instead of only serving those data as the /versions API output.

This is a config file, to say: what are RN and Expo versions we support to looking up.

What happens if package uses version outside this list? Does it need to be in file at all, can't we just use the version fields from JSON to establish those lists dynamically?

---

Also there is something important that might not be communicated well, but the committed data.json is just the local test stub file. We do not store any "real/production" data in the repository, so there is no need to commit all of those new files, we should retain only few, required for testing the functionality.

Saying that, I think that we should still consider migrating to DB first, if possible, which will make managing data and future development on new features way easier. 🙂

[giautm]

@Simek : I update the format of file by using the new entry in data.json rnVersions. I complete some simple logic to lookup the dependency. I was tested with bellow request and noted:

curl --location --request POST 'http://localhost:3000/api/versions' \
--header 'Content-Type: application/json' \
--data-raw '{"reactNativeVersion":"0.68.0","packages":["expo-av", "@stripe/stripe-react-native", "react-navigation-header-buttons"]}'

This request returns:

{
    "reactNativeVersion": "0.68.0",
    "packages": [
        {
            "npmPackage": "react-navigation-header-buttons",
            "versionRange": "9.0.1"
        },
        {
            "npmPackage": "expo-av",
            "versionRange": "9.0.0" <---- Incorrect version
        },
        {
            "npmPackage": "@stripe/stripe-react-native",
            "versionRange": "0.9.0"
        }
    ]
}

But, the request with expoSdkVersion will return

curl --location --request POST 'http://localhost:3000/api/versions' \
--header 'Content-Type: application/json' \
--data-raw '{"reactNativeVersion":"0.68.0","expoSdkVersion":"45.0.0","packages":["expo-av", "@stripe/stripe-react-native", "react-navigation-header-buttons"]}'

{
    "reactNativeVersion": "0.68.0",
    "expoSdkVersion": "45.0.0",
    "packages": [
        {
            "npmPackage": "expo-av",
            "sdkVersion": "45.0.0",
            "versionRange": "~11.2.3"
        },
        {
            "npmPackage": "@stripe/stripe-react-native",
            "sdkVersion": "45.0.0",
            "versionRange": "0.6.0"
        },
        {
            "npmPackage": "react-navigation-header-buttons",
            "versionRange": "9.0.1"
        }
    ]
}

Because the expo-av not add react-native in dependencies/ peerDependencies anymore - since the version 9.0.0.

I still call two endpoints of Expo to avoid copying the data to here then it out-of-sync from the original. When we set up a database/private endpoint for sync data from the expo project. I will remove the call to those endpoints.

cc @Simek @EvanBacon @brentvatne.

[brentvatne]

hey @giautm! thanks for putting this together. could you possible update the PR description to include some more information about how this all works?

some comments:

determining a compatible react-native version

from reading the code i can get an idea but i'd like to see a written description of the intent and reasoning behind it. it looks like we chose to use devDependencies, peerDependencies, and dependencies as the basis for a compatible version - this seems reasonable but as you pointed out with the case of expo-av, this won't always be useful.

i am somewhat concerned about encouraging developers to depend on expressing react-native version compatibility via peer dependencies, because this means that they need to release a new version of their library for every new react-native version or risk npm 7+ possibly installing an old version of react-native automatically. most libraries don't release new versions every react-native release, and that's fine, but if everyone uses peer dependencies this could become a problem. dev dependencies seems mostly reasonable.. but what if you're working on supporting react-native 0.69 in your module but still support 0.68? as for the plain dependencies field - i don't think any library should ever include react-native as an actual dependency.

we could possibly use these values as a best-guess but we may want to introduce a specific field that is only intended for the purpose of telling react native directory what react-native versions are compatible. i'm open to being convinced that this isn't needed though. just concerned about overloading behavior on existing fields as described above.

rate limiting

do you think we are going to get rate limited by npm here? each package version lookup seems to be resulting in two more requests to npm.

[giautm]

Thank @brentvatne for your comment.

Determining a compatible react-native version

yes, guess what react-native version is supported based on dependencies should be for reference only as we have no other information. In the future, we may describe a special field to report that. Or, we can also use the information from the rnx-kit field to know the supported version.

package.json:

{
  "name": "example-a",
  "version": "0.2.0",
  "@react-native-community/directory": {
    "expoSdkVersion": "45.0.0",
    "reactNativeVersion": "0.58.0"
  }
}

The steps will be: check the @react-native-community/directory field -> check the rnx-kit field -> check dependencies.

However, I still think we should have a database, where people can announce the version and their support, and update automatically every time a new version is released.

Rate limiting

Scraping information from NPM should be done only when we first deploy the database, and then updated via webhooks.

[brentvatne]

thank you for the clarifications @giautm! this sounds very reasonable.

However, I still think we should have a database, where people can announce the version and their support, and update automatically every time a new version is released.

i agree! would you be able to write up a draft of how this might work? i'm curious to see the vision end to end

[Simek]

I have checked out the branch locally and play with a feature a bit, here is my feedback:

• versions endpoint should use GET instead of POST for the API uniformity and ease of use
• it looks like rnVersions list in library do not include correct data, the version is set to the same value for all debug libraries
  
  Also, probably we can skip including minor releases on the list. (API data looks correct)
• rnVersions is sometimes an empty object, in those cases it would be nice to just skip the field completely, so it would be easy to check if data is present
• none of the debug packages included expoSdkVersion field in the library avaible on the website
• using non existent or not declared in config list RN or Expo versions in API still returns data, it would be nice to return 400 in cases like that with a reason message
  
• when required parameters are missing we also should return 400, instead of returning almost empty response
  
• it would be nice to document new endpoint in the project Redme file

[giautm]

The idea behind the New Version Announcement endpoint is to take in the name of the package, then we're going to scrape the information from NPM, as a trusted source. To avoid users intentionally changing the version of a library not owned by them. And also avoid having to allocate JWT Token for each repository to update their own package.

[brentvatne]

hey @giautm! sorry for the delay on review here, i've been in and out of the office. i just spoke with @kelset and @tido64 from microsoft about this and they are excited and going to take a quick look when they have the chance :)

[brentvatne]

i believe that we can ship this in its current form, then follow up with further refinements eg: moving to a proper db, being able to announce updates via an endpoint

[Simek]

@brentvatne As we speak lately on the Conf, the only blocker form the list for me is the method used for the API calls.

Other things are not major, however still most of them are valid in my eyes and I would like to see them addressed in the future too. @giautm if you have another opinion on some parts, please let me know, I'm sure we can find the way to make most of the feature pod the CLI and Directory side.

[brentvatne]

ah yes, right @Simek - i forgot about that. let's switch to GET for the versions endpoint @giautm

[kelset]

hey folks, we are in the process of reworking how things work in rnx-kit around declaring dependencies/version of react-native for dep-check (see here microsoft/rnx-kit#1757) - so I feel that in order to unblock this PR from our work it'd probably be better to strip out thernx-kit logic and then potentially revisit this when dep-check 2.0 is rolled out.

Sorry for the delay in replying here btw 🥲

[brentvatne]

@giautm - I think we can ship this if we update to use GET instead of POST - let me know if you'd like someone to take this PR over or if you're still interested in working on it :)

[giautm]

@giautm - I think we can ship this if we update to use GET instead of POST - let me know if you'd like someone to take this PR over or if you're still interested in working on it :)

Working on it, sorry about the delay

[kelset]

(btw just for cross-reference, this might become overtime - maybe in a second iteration - related to microsoft/rnx-kit#1863 )

[Simek]

@giautm We already have a check for the missing entry in NPM registry:

directory/scripts/fetch-npm-data.js

Lines 55 to 60 in 71e6d86

	if (!downloadData.downloads) {
	console.warn(
	`[NPM] ${npmPkg} doesn't exist on npm registry, add npmPkg to its entry or remove it!`
	);
	return { ...pkgData, npm: null };
	}

Can you explain what issue you are having with Babylon?

Existence in NPM registry is perceived as a required, we stopped to throw the build in those case due to templates, which are mostly not published. Babylon addition required changing the GitHub URL regex due to uncommon monorepo structure, maybe we miss something more?

[giautm]

Can you explain what issue you are having with Babylon

This is the error related to BabylonJS/BabylonReactNative

[GH] Retrying fetch for https://github.com/BabylonJS/BabylonReactNative/tree/master/Modules/%40babylonjs/react-native TypeError: Cannot read properties of undefined (reading 'committedDate')
    at createRepoDataWithResponse (/react-native-libraries/scripts/fetch-github-data.js:283:70)
    at _callee5$ (/react-native-libraries/scripts/fetch-github-data.js:213:20)
    at tryCatch (/react-native-libraries/scripts/fetch-github-data.js:40:2404)
    at Generator._invoke (/react-native-libraries/scripts/fetch-github-data.js:40:1964)
    at Generator.next (/react-native-libraries/scripts/fetch-github-data.js:40:3255)
    at asyncGeneratorStep (/react-native-libraries/scripts/fetch-github-data.js:42:103)
    at _next (/react-native-libraries/scripts/fetch-github-data.js:44:194)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

[Simek]

Thanks for the details! 👍

Just want to let you know that I have fixed the Babylon issue, but it looks like you have already figure it out too. 🙂

Also the cleanup script requires a change, otherwise it was cleaning npmPkg field for the entry which leads to NPM fetch failure. We require this value for the monorepos anyway, the script was ignoring that fact so far, but it was not a problem since we did not have @ in the directory name.