GoodDAPP icon indicating copy to clipboard operation
GoodDAPP copied to clipboard

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

Open L03TJ3 opened this issue 1 year ago • 9 comments

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

L03TJ3 avatar Dec 13 '23 14:12 L03TJ3

@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

sirpy avatar Dec 13 '23 21:12 sirpy

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

L03TJ3 avatar Dec 14 '23 04:12 L03TJ3

@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 avatar Dec 16 '23 12:12 johnsmith-gooddollar

@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 avatar Dec 18 '23 11:12 johnsmith-gooddollar

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

johnsmith-gooddollar avatar Dec 18 '23 11:12 johnsmith-gooddollar

@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 avatar Jan 18 '24 03:01 L03TJ3

@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

johnsmith-gooddollar avatar Jan 18 '24 04:01 johnsmith-gooddollar

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

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

L03TJ3 avatar Jan 18 '24 09:01 L03TJ3

@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

johnsmith-gooddollar avatar Jan 18 '24 09:01 johnsmith-gooddollar