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

Chore/bug: bumping sdk-v2 to latest breaks tests #4161

Closed
L03TJ3 opened this issue Dec 13, 2023 · 9 comments
Closed

Chore/bug: bumping sdk-v2 to latest breaks tests #4161

L03TJ3 opened this issue Dec 13, 2023 · 9 comments
Assignees

Comments

@L03TJ3
Copy link
Collaborator

L03TJ3 commented Dec 13, 2023

Description

After bumping sdk-v2 to latest tests start failing because of deps not being optimized/ignored/build correclty.
I attempted to add some to the test ignore pattern here:

'<rootDir>/node_modules/(?!(jest-)?react-native|react-navigation|react-navigation-redux-helpers|react-phone-number-input|webrtc-adapter|@gooddollar/react-native-facetec|@ceramicnetwork|@web3-onboard|axios)',

But I kept adding more and more, didn't feel like a 'solution'
Currently we only use the bridge-contracts and flow from sdk-v2, and these contracts are at the latest version available.

but this should be handled/fixed before we do need some later versions of sdk-v2

Steps to reproduce

  1. bump sdk-v2 to latest
  2. run tests (there are various failing, you check this run for some of them: https://github.com/GoodDollar/GoodDAPP/actions/runs/7194773244
@sirpy
Copy link
Contributor

sirpy commented Dec 13, 2023

@L03TJ3 from what I see the problem seems to be orbis, is that what changed since last sdk-v2 version in gooddapp?
Also I see in the error message a link to:
https://jestjs.io/docs/ecmascript-modules

@L03TJ3
Copy link
Collaborator Author

L03TJ3 commented Dec 14, 2023

Yes it started with orbis is, but after there came some more. Since it was not part of the PR where I first noticed it I put in a ticket.

Thanks didn't spot the link

@johnsmith-gooddollar
Copy link
Collaborator

@L03TJ3 @sirpy adding all three @ceramicnetwork|@orbisclub|@didtools solves the invalid token issue
But then we receive "cannot find module" error. Also they're reproduces when you're trying to run app locally
(did you tried to do it @L03TJ3 ?)

The reason is that after update a lot of the modules were updated to es6-only versions (having type=module and exports sections in package.json).

I tried solved it by patching libraries (adding main/types fallbacks for es5, adding js files to the root re-exporting sumbodules being required from root) but this isn't really a solution.
a) it requires patching 8 libraries
b) it does not solves everything. app builds but then we have errors related to BigInt. so the new libraries versions aren't compatible with the app still using BN instead of BigInt

As an quick solution I suggest:

  • revert and fixate ceramicnetwork to the following versions at the sdk-v2 side:
    "@ceramicnetwork/http-client": "^2.0.4",
    "@ceramicnetwork/stream-tile": "^2.1.3",
  • revert and fixate orbis to the version requiring ceramic having version from above

In the perspective I suggest to upgrade GooDDapp to webpack5 / babel7 having es6-only modules support.
And also we would need to stop using BN / web3, work with the sdk methods using ethers instead.
And of course migrate to ethers6

@johnsmith-gooddollar
Copy link
Collaborator

@L03TJ3 also I suggest You do not use whole Orbis SDK, if you want just read posts. here is a function copied from Orbis source which load posts. It requires just one extra dev instead of a lot of deps Orbis brings to the project:

import { memoize } from "lodash"
import { createIPFSLoader, isValidCID } from "ipfs-utils"

import { FeedConfig, FetcherFactory, StreamFetcher, StreamPost } from "../types"

export type G$FeedConfig = FeedConfig & {
  context: string
  tag?: string
  ipfsGateways?: string
}

const indexerUrl = "https://ylgfjdlgyjmdikqavpcj.supabase.co"
const indexerKey =
  "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6InlsZ2ZqZGxneWptZGlrcWF2cGNqIiwicm9sZSI6ImFub24iLCJpYXQiOjE2NTQ3NTc3NTIsImV4cCI6MTk3MDMzMzc1Mn0.2XdkerM98LhI6q5MBBaXRq75yxOSy-JVbwtTz6Dn9d0"

const filterDefaults = {
  q_did: null,
  q_only_master: false,
  q_contexts: null,
  q_master: null,
  q_reply_to: null,
  q_include_child_contexts: false,
  q_term: null,
  q_is_reply: null,
  q_is_repost: null,
}

const getIndexer = memoize(async () => {
  const { createClient } = await import("@supabase/supabase-js")

  return createClient(indexerUrl, indexerKey)
})

const getPosts = async (
  filter: Pick<G$FeedConfig, "context" | "tag">,
  limit: number,
  offset?: number,
) => {
  const skip = offset ?? 0
  const { context, tag } = filter
  const api = await getIndexer()

  return api
    .rpc("default_posts_09", {
      ...filterDefaults,
      q_context: context ?? null,
      q_tag: tag ?? null,
    })
    .range(skip, skip + limit - 1)
    .order("timestamp", { ascending: false })
    .then(
      ({ data }) =>
        data as {
          stream_id: string
          content: {
            title: string
            body: string
            data: unknown
          }
        }[],
    )
}

export const createG$Fetcher: FetcherFactory<G$FeedConfig> = <
  T extends StreamPost = StreamPost,
>({
  context,
  tag,
  ipfsGateways,
}: G$FeedConfig): StreamFetcher<T> => {
  const loadPicture = createIPFSLoader(ipfsGateways)

  return async (limit: number, offset?: number) => {
    const filter = { context, tag }
    const orbisPosts = await getPosts(filter, limit, offset)

    const streamPosts = (orbisPosts ?? []).map(
      ({ stream_id, content: { title, body, data } }) => ({
        ...(data as T),
        id: stream_id,
        title: title,
        content: body,
      }),
    )

    return Promise.all(
      streamPosts.map(async (post) => {
        const { picture } = post

        return isValidCID(picture)
          ? { ...post, picture: await loadPicture(picture) }
          : post
      }),
    )
  }
}

@johnsmith-gooddollar
Copy link
Collaborator

johnsmith-gooddollar commented Dec 18, 2023

if you need also getPost method You could also copy it from Orbis source:

https://github.com/OrbisWeb3/orbis-sdk/blob/master/index.js#L1495

@L03TJ3 L03TJ3 self-assigned this Jan 11, 2024
@L03TJ3
Copy link
Collaborator Author

L03TJ3 commented Jan 18, 2024

@sirpy @johnsmith-gooddollar
Came to the same conclusion as @johnsmith-gooddollar that the most complete solution is upgrade to webpack5 as all the issues trace back to mostyl "exports" fields which are not supported in webpack 4 breaking resolutions (I tried using aliases but apparently the patches @johnsmith-gooddollar would still be required for some)

@sirpy said in a sync that it might be a bit of work to do this upgrade

So what are we going to do?

  1. do the downgrade to older @ceramicnetwork packages (only solving it for this particular dep tree, and doesn't actually solve anything) + this will stay potentional chore work whenever newer deps are pulled in
  2. do the upgrade to webpack 5 (Its the actual solution and might be worth the work since we are doing the planning for merging certain flows between dapp/wallet with new screens/features build in our mono-repo so this resolution issue might be coming back way more often)

@L03TJ3 L03TJ3 added the TBD Issue needs defining label Jan 18, 2024
@johnsmith-gooddollar
Copy link
Collaborator

@L03TJ3 I suggest to try this #4161 (comment)

because we actually do not need whole Orbis, just 1 or 2 functions (getPosts/getPost)
no Orbis => no ceramic
no ceramic => no webpack issues

@L03TJ3
Copy link
Collaborator Author

L03TJ3 commented Jan 18, 2024

@johnsmith-gooddollar ah okay, yes that would maybe be simpler for today then

Do we need to make webpack upgrade a new issue though?

@johnsmith-gooddollar
Copy link
Collaborator

@L03TJ3 for now let's just try this fix please, then will talk regarding web pack etc
also I wanter to bump ethers, remember ? and this might make package incompatible with usedapp, if they still not migrated to ethers6. let's discuss all this later, now will concentrate on this issue and will try the fix I suggested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants