GoodDollar / GoodDAPP

GoodDollar.org Wallet is the simplest access point to Claim your daily G$. It Is based on web3 and React native web.
good-dapp.vercel.app
MIT License
100 stars 53 forks source link

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

Closed L03TJ3 closed 4 months ago

L03TJ3 commented 8 months ago

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: https://github.com/GoodDollar/GoodDAPP/blob/ae4d3c2358548166fa14059f8718f0a732a2dabc/config-overrides.js#L37

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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

@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:

    "@ceramicnetwork/http-client": "^2.0.4",
    "@ceramicnetwork/stream-tile": "^2.1.3",

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 commented 8 months ago

@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 commented 8 months ago

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 commented 7 months ago

@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)
johnsmith-gooddollar commented 7 months ago

@L03TJ3 I suggest to try this https://github.com/GoodDollar/GoodDAPP/issues/4161#issuecomment-1860215389

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 commented 7 months ago

@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 commented 7 months ago

@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