feat(pricefeed) Migrate Price Feed IDs to documentation

[aditya520]

TODOs

• Stying for Dark mode.
• Styling for Price Feed IDs.
• Add instructions and some callouts for the reader on utilizing these IDs.
• Add Beta Price Feed IDs.
• Add Solana Price Feed Accounts.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	❌ Failed (Inspect)			Nov 14, 2024 3:08am
documentation	❌ Failed (Inspect)			Nov 14, 2024 3:08am

[cprussin]

I think you can get rid of mui entirely here; everything I see you doing with it is trivial to do with tailwind, or there are already built-in components in nextra or in the app that you can use instead.

Additionally, please clean up the state management for the asset types list as per my comment. Make sure to properly handle loading and error statesat least, and ideally also follow some of the best practices I shared in that comment.

[cprussin]

Mentioned this in slack but we should try to use built-in nextra components where possible. I don't really have an issue with using mui for stuff that isn't in nextra, but I also don't think you get much out of the stuff you're using here and can probably do away with mui entirely.

[cprussin]

I don't see anywhere that you're actually using emotion, is this a peer dependency of mui or something?

[aditya520]

https://stackoverflow.com/a/69632381
Yes it is.

[cprussin]

That stackoverflow just tells me that they're using emotion, not that it's a peer dependency. There shouldn't be any reason to add emotion to our dependencies unless if either:

1. you need to use it directly (in general you shouldn't, we have tailwind for this, the only possible exception would be if you need to pass overrides to a library that internally uses it)
2. it's a peer dependency of a package you depend on (what this means is that the package has it listed in it's peerDependencies list in the package.json, indicating that the dependency must be installed by the consumer; typically this only happens for things like plugins defining what they are plugging into or things like React where there can only be one copy of the dependency in the build graph)

[cprussin]

There's a few issues with this effect and the state it creates:

1. It's not accurate to initialize priceFeeds to an empty array on line 24, you should properly handle loading states and represent that state differently from a loaded array.
2. You aren't actually using priceFeeds anywhere except to get the list of asset types. You should extract the asset types and dedupe them when you load the data and store that in the state, rather than deduping in the render path on line 76 below.
3. The transformation on lines 33 - 37 is IMO not useful; you're basically just changing snake case to camel case. IMO there isn't any serious benefit to doing that, and there are costs (added cognitive overhead, makes the code harder to search, etc). I know that camel case is the convention, but sticking to convention is not worth added complexity and making the code harder to search.
4. You should always use a catch any time you call a promise without using await, e.g. on line 41. Omitting the catch means uncaught exceptions could potentially put the app into an unstable state; additionally here you should make the UI indicate errors properly.
5. I always strongly recommend extracting functions like fetchPriceFeeds outside the closure scope. For one, it makes the code simpler and easier to follow; for two it's easy to introduce performance issues by creating closures in a hot path by accident.
6. You should always prefer ?? over || (on lines 35 and 36), see https://typescript-eslint.io/rules/prefer-nullish-coalescing/.
7. Minor nit but in es6 you can do [...new Set(...)] instead of Array.from(new Set(...)) to dedupe, the former is considered more idiomatic.
8. You can use the "fake sum types" pattern I shared to encapsulate the loading/error state management here if you like.

Putting all this together you can change the component to something like this:

export const PriceFeedTable = () => {
  const [selectedAssetType, setSelectedAssetType] = useState("All");

  // ...
  <AssetTypesSelect
    selectedAssetType={selectedAssetType}
    setSelectedAssetType={setSelectedAssetType}
  />
}

type AssetTypesSelectProps = {
  selectedAssetType: string;
  setSelectedAssetType: (value: string) => void;
}

const AssetTypesSelect = ({ selectedAssetType, setSelectedAssetType }: AssetTypesSelectProps) => {
  const assetTypes = useAssetTypes();

  switch (state.type) {
    case AssetTypesStateType.NotLoaded:
    case AssetTypesStateType.Loading: {
      // use any spinner we're already using in the app anywhere else...
      return <Spinner /> 
    }
    case AssetTypesStateType.Error: {
      return <p>Error loading asset types!</p>;
    }
    case AssetTypesStateType.Loaded: {
      return (
        <Select
          value={selectedAssetType}
          onChange={(e) => setSelectedAssetType(e.target.value)}
          size="small"
          sx={{ width: 200 }}
        >
          <MenuItem value="All">All Asset Types</MenuItem>
          {state.assetTypes.map(assetType => (
            <MenuItem key={assetType} value={assetType}>
              {assetType ?? "Uncategorized"}
            </MenuItem>
          ))}
        </Select>
      );
    }
  }
};

const useAssetTypes = (): AssetTypesState => {
  const [assetTypes, setAssetTypes] = useState<AssetTypesState>(AssetTypesState.NotLoaded());

  useEffect(() => {
    setAssetTypes(AssetTypesState.Loading());
    fetchAssetTypes()
      .then(assetTypes => setAssetTypes(AssetTypesState.Loaded(assetTypes)))
      .catch(error => setAssetTypes(AssetTypesState.Error(error)));
  }, []);

  return assetTypes;
}

const fetchAssetTypes = async () => {
  const hermesClient = new HermesClient("https://hermes.pyth.network");
  const feeds = await hermesClient.getPriceFeeds();
  return [...new Set(feeds.map(feed => feed.attributes.asset_type ?? ""))];
};

enum AssetTypesStateType {
  NotLoaded,
  Loading,
  Loaded,
  Error
}

const AssetTypesState = {
  NotLoaded: () => ({ type: AssetTypesStateType.NotLoaded as const }),
  Loading: () => ({ type: AssetTypesStateType.Loading as const }),
  Loaded: (assetTypes: string[]) => ({ type: AssetTypesStateType.Loaded as const, assetTypes }),
  Error: (error: unknown) => ({ type: AssetTypesStateType.Error as const, error }),
}

type AssetTypesState = typeof AssetTypesState[keyof typeof AssetTypesState];

[aditya520]

Replying to point 3.
I am using PriceFeeds variable to filter here

    const matchesSearch =
      feed.id.toLowerCase().includes(searchTerm.toLowerCase())

and then to display the data. What should be the best way around?

[cprussin]

Ah, apologies, I was so focused on the asset types that I didn't think about the other usage of the feeds. In my suggested code, just replace fetchAssetTypes with:

const fetchPriceFeeds = async () => {
  const priceFeeds = await new HermesClient("https://hermes.pyth.network").getPriceFeeds();
  const assetTypes = [...new Set(priceFeeds.map(feed => feed.attributes.asset_type ?? ""))];
  return { priceFeeds, assetTypes };
};

Then update variable names etc as appropriate to reflect that the state is storing the price feeds and not just asset types (e.g. rename useAssetTypes to usePriceFeeds). Then update the component to e.g.:

export const PriceFeedTable = () => {
  const [selectedAssetType, setSelectedAssetType] = useState("All");
  const priceFeedsState = usePriceFeeds();

  // ...
  <AssetTypesSelect
    priceFeedsState={priceFeedsState}
    selectedAssetType={selectedAssetType}
    setSelectedAssetType={setSelectedAssetType}
  />
}

type AssetTypesSelectProps = {
  priceFeedsState: PriceFeedsState;
  selectedAssetType: string;
  setSelectedAssetType: (value: string) => void;
}

const AssetTypesSelect = ({ priceFeedsState, selectedAssetType, setSelectedAssetType }: AssetTypesSelectProps) => {
  switch (priceFeedsState.type) {
    // ...
  }
}

Let me know if this makes sense.

[cprussin]

I don't think there's a great reason to use MUI for text fields or selects; we already have other components in the codebase for both cases, see e.g. https://docs.pyth.network/price-feeds/api-reference/cosmwasm/query-price-feed and the components on that page (Selector.tsx for the select and Input.tsx for the text input)

[cprussin]

The Box component isn't useful with tailwind; these styles can be directly added to a div by using the correct utility classes, e.g. something like <div className="mb-1 flex gap-2">

[cprussin]

You can avoid having to use this component by using the CopyToClipboard nextra component which has feedback built in and is the same component that's used for code block copying