PKG -- [fcl-core] split fcl to fcl-core, fcl, and fcl-react-native #1809
Conversation
🦋 Changeset detectedLatest commit: f2e1cff The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/fcl/src/fcl.ts
Outdated
| @@ -1,10 +1,9 @@ | |||
| export * from "./shared-exports" | |||
| export * from "@onflow/fcl-core" | |||
There was a problem hiding this comment.
Can we namespace the exports from FCL core? i.e. separate the ones that are supposed to be exported/"shared exports" between both packages & the ones that are only used internally.
That way way we avoid external consumers coupling with FCL internals (otherwise some exports like getMutate, etc. will be exported from @onflow/fcl
| }, | ||
| "dependencies": { | ||
| "@babel/runtime": "^7.18.6", | ||
| "@onflow/fcl-core": "^1.8.1", |
There was a problem hiding this comment.
imo we should lock this dependency (i.e. "1.8.1"). I have the feeling we're going to have a difficult time following semver with this package since this is was previously functionality internal to FCL (@onflow/fcl-react-native will probably end up tightly coupled/reliant on an exact version of @onflow/fcl-core).
Also if it's not locked we risk users encountering different "FCL" behaviour in the same version of FCL. This is honestly a problem with all of our packages and I wonder if we should enforce this requirement across the board (but this is a different discussion maybe).
packages/fcl/package.json
Outdated
| "start": "fcl-bundle --watch", | ||
| "lint": "eslint ." | ||
| }, | ||
| "dependencies": { | ||
| "@babel/runtime": "^7.18.6", | ||
| "@onflow/fcl-core": "^1.8.1", |
There was a problem hiding this comment.
same comment about versioning/locking dependencies
nialexsan
changed the title
There was a problem hiding this comment.
LGTM. I think as long as @onflow/fcl is still working we're good here. I left a couple other comments as some ideas we can follow up on but I don't think it's important for this PR and we can merge this so that we don't start getting more merge conflicts.
Awesome stuff :)
| @@ -1,6 +1,6 @@ | |||
| import * as Linking from "expo-linking" | |||
| import {AppState} from "react-native" | |||
| import {URL} from "../url" | |||
| import {URL} from "@onflow/fcl-core" | |||
There was a problem hiding this comment.
I think our URL patch might be able to be replaced by adding react-native-url-polyfill as a dependency in fcl-react-native now that we made the split actually. But not a big deal, we can follow this stuff up with another PR with the rest of the react native changes imo
| "peerDependencies": { | ||
| "@react-native-async-storage/async-storage": "^1.13.0", | ||
| "expo-linking": "^4.0.0", | ||
| "expo-web-browser": "^12.0.0", | ||
| "react": "^17.0.0 || ^18.0.0", | ||
| "react-native": "^0.0.0-0 || 0.60 - 0.72 || 1000.0.0" | ||
| } |
There was a problem hiding this comment.
When we make follow up changes to fcl-react-native we can try to clean up these peer dependencies & see which ones we can transition to regular dependencies.
@onflow/fcl-core- shared code@onflow/fcl- fcl for web@onflow/fcl-react-native- fcl for react native