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

fix(explorer): various fixes #3195

Merged
merged 11 commits into from
Sep 19, 2024
Merged

fix(explorer): various fixes #3195

merged 11 commits into from
Sep 19, 2024

Conversation

holic
Copy link
Member

@holic holic commented Sep 18, 2024

  • made observer() options arg optional
  • fixed favicon
  • fixed regression in explorer vs internal layouts
  • fixed redirects
  • fixed issues with validating chain IDs and names
  • reworked iframe rendering to reuse existing iframes for the same url

Copy link

changeset-bot bot commented Sep 18, 2024

🦋 Changeset detected

Latest commit: 0779301

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 26 packages
Name Type
@latticexyz/explorer Patch
@latticexyz/abi-ts Patch
@latticexyz/block-logs-stream Patch
@latticexyz/cli Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/dev-tools Patch
@latticexyz/faucet Patch
@latticexyz/gas-report Patch
@latticexyz/protocol-parser Patch
@latticexyz/react Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/stash Patch
@latticexyz/store-indexer Patch
@latticexyz/store-sync Patch
@latticexyz/store Patch
@latticexyz/utils Patch
@latticexyz/world-module-metadata Patch
@latticexyz/world-modules Patch
@latticexyz/world Patch
mock-game-contracts Patch
ts-benchmarks Patch

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

alvrs
alvrs previously approved these changes Sep 18, 2024
@holic holic changed the title fix(explorer): default opts to empty object fix(explorer): various fixes Sep 18, 2024
Copy link
Contributor

@karooolis karooolis Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See it has been changed back to icon.svg. Would be nice to use layout's metadata but can't figure out quickly how to make that work from a non-root path so I'm fine with it, especially since it meets next.js convention.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I couldn't figure out why specifying the path in page meta lead to a bunch of invalid requests to the icon, even though the meta tag rendered fine and had the correct path

changing this to just icon.svg follows the next.js convention which serves the icon automatically: https://nextjs.org/docs/app/api-reference/file-conventions/metadata/app-icons#image-files-ico-jpg-png

export const supportedChainsById = Object.fromEntries(
Object.entries(supportedChains).map(([, chain]) => [chain.id, chain]),
);
export type supportedChains = typeof supportedChains;
Copy link
Contributor

@karooolis karooolis Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if it should start uppercase given it's a type, and that's been the convention so far? Same for supportedChainName and supportedChainId

Copy link
Member Author

@holic holic Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just a shortcut to avoid having to sprinkle typeof everywhere you're using it with a const

when the name mirrors the type, you can use the same name/reference in both types and runtime (we borrowed this pattern from arktype and use it throughout config etc.)

karooolis
karooolis previously approved these changes Sep 19, 2024
@holic holic merged commit 55ae822 into main Sep 19, 2024
11 of 12 checks passed
@holic holic deleted the holic/observer-arg branch September 19, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants