Skip to content

Conversation

@shiyasmohd
Copy link
Contributor

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Jan 31, 2025

🦋 Changeset detected

Latest commit: 40bf36d

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

This PR includes changesets to release 1 package
Name Type
@graphprotocol/graph-cli 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

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 31, 2025

Deploying graph-tooling with  Cloudflare Pages  Cloudflare Pages

Latest commit: 40bf36d
Status: ✅  Deploy successful!
Preview URL: https://1535b216.graph-tooling.pages.dev
Branch Preview URL: https://shiyasmohd-int8-as-id-types.graph-tooling.pages.dev

View logs

@github-actions
Copy link
Contributor

github-actions bot commented Jan 31, 2025

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphprotocol/graph-cli 0.96.0-alpha-20250227123538-40bf36d4620a4188f5abab832dfa8c659876774b npm ↗︎ unpkg ↗︎

@shiyasmohd shiyasmohd self-assigned this Feb 1, 2025
@YaroShkvorets
Copy link
Collaborator

Is this PR finished?
graph build with these changes doesn't work if id type is set to Int8.
Codegen needs to be adjusted.

@shiyasmohd
Copy link
Contributor Author

@YaroShkvorets PR is finished now

@YaroShkvorets YaroShkvorets self-requested a review February 11, 2025 02:51
@YaroShkvorets
Copy link
Collaborator

YaroShkvorets commented Feb 11, 2025

@shiyasmohd
Codegen now works, but scaffolded subgraph doesn't.

Steps to reproduce:

  1. Init new subgraph:
graph init test test --from-contract 0xdAC17F958D2ee523a2206206994597C13D831ec7 --protocol ethereum --network mainnet --index-events
  1. Replace id: Bytes! for Issue entity in the GraphQL schema with id: Int8!
  2. Run graph codegen - works
  3. Run graph build - fails with
 Compile subgraphERROR TS2322: Type '~lib/@graphprotocol/graph-ts/common/collections/Bytes' is not assignable to type 'i64'.

     event.transaction.hash.concatI32(event.logIndex.toI32())
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 in src/contract.ts(30,5)

Basically need to generate different code for Int8 that constructs unique 64-bit id for our event:

let entity = new Issue(
    event.transaction.hash.concatI32(event.logIndex.toI32())
  )

I guess we could just hash this thing and take first 8 bytes in these cases.

Or we could construct something from block number, transaction index and event index.


UPD: nevermind, it's irrelevant since we scaffold Bytes ids by default, so can leave it to the subgraph developer to decide if they change id type to Int8.

Copy link
Collaborator

@YaroShkvorets YaroShkvorets left a comment

Choose a reason for hiding this comment

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

One last thing, can we add a test case in src/codegen/schema.test.ts

static STRING = Symbol('String');
static INT8 = Symbol('Int8');

private kind: typeof IdField.BYTES | typeof IdField.STRING;
Copy link
Collaborator

Choose a reason for hiding this comment

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

add | typeof IdField.INT8

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is meant for line 21

case 'Int8':
this.kind = IdField.INT8;
break;
default:
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's add case 'string' for consistency and future refactoring

@shiyasmohd shiyasmohd force-pushed the shiyasmohd/int8-as-id-types branch from c6e324e to 40bf36d Compare February 27, 2025 12:35
@shiyasmohd
Copy link
Contributor Author

@YaroShkvorets PR is ready for review

@YaroShkvorets YaroShkvorets merged commit 30f2aa5 into main Feb 27, 2025
9 checks passed
@YaroShkvorets YaroShkvorets deleted the shiyasmohd/int8-as-id-types branch February 27, 2025 19:20
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