-
Notifications
You must be signed in to change notification settings - Fork 181
moves ofac and protocol store #1012
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,10 @@ const extraNodeModules = { | |
| sdkAlphaPath, | ||
| 'dist/esm/constants/analytics.js', | ||
| ), | ||
| '@selfxyz/mobile-sdk-alpha/stores': path.resolve( | ||
| sdkAlphaPath, | ||
| 'dist/esm/stores.js', | ||
| ), | ||
| // Main exports | ||
| '@selfxyz/common/utils': path.resolve( | ||
| commonPath, | ||
|
|
@@ -163,6 +167,10 @@ const extraNodeModules = { | |
| commonPath, | ||
| 'dist/esm/src/utils/csca.js', | ||
| ), | ||
| '@selfxyz/common/utils/ofac': path.resolve( | ||
| commonPath, | ||
| 'dist/esm/src/utils/ofac.js', | ||
| ), | ||
|
Comment on lines
+170
to
+173
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify compiled entry exists for '@selfxyz/common/utils/ofac' Ensure dist/esm/src/utils/ofac.js is part of the common package build outputs. 🏁 Script executed: #!/bin/bash
ls -l ../common/dist/esm/src/utils/ofac.js || echo "ofac.js missing"Length of output: 99 🏁 Script executed: #!/bin/bash
find ../common/dist -type f | grep -Ei 'ofac\.(js|mjs|cjs)$' || echo "no compiled ofac file found"Length of output: 77 Alias for '@selfxyz/common/utils/ofac' points to a non-existent file 🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. double check this. it should after we build the package
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain@aaronmgdr You're absolutely right! Let me verify the build configuration to confirm the OFAC utility will be properly built. 🏁 Script executed: Length of output: 2918 @aaronmgdr You're absolutely correct! The build configuration is properly set up: ✅ Source file exists: The verification agent was checking the current state (before build), but your configuration will generate The OFAC utility migration looks good to go! 🚀 🧠 Learnings used |
||
| // Types subpaths | ||
| '@selfxyz/common/types/passport': path.resolve( | ||
| commonPath, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,9 @@ | |
| "@selfxyz/mobile-sdk-alpha/constants/analytics": [ | ||
| "../packages/mobile-sdk-alpha/dist/esm/constants/analytics.js" | ||
| ], | ||
| "@selfxyz/mobile-sdk-alpha/stores": [ | ||
| "../packages/mobile-sdk-alpha/dist/esm/stores.js" | ||
| ], | ||
|
Comment on lines
+20
to
+22
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEnsure type declarations exist for the new alias target The alias maps directly to dist/esm/stores.js. If dist/esm/stores.d.ts isn’t emitted, consumers lose types and IDE tooling. Run: If stores.d.ts is missing, ensure tsup/tsconfig emits declarations for the stores entry. 🏁 Script executed: #!/bin/bash
# Verify compiled JS and DTS exist for the alias
ls -l ../packages/mobile-sdk-alpha/dist/esm/stores.js || echo "stores.js missing"
ls -l ../packages/mobile-sdk-alpha/dist/esm/stores.d.ts || echo "stores.d.ts missing"Length of output: 229 Fix missing build artifacts for 🤖 Prompt for AI Agents |
||
| "@/*": ["./src/*"] | ||
| } | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||
| { | ||||||||||||||
| "name": "@selfxyz/mobile-sdk-alpha", | ||||||||||||||
| "version": "2.6.4", | ||||||||||||||
| "version": "0.0.1", | ||||||||||||||
| "description": "Self SDK (alpha) for registering and proving. Adapters-first, RN-first with web shims.", | ||||||||||||||
| "keywords": [ | ||||||||||||||
| "self", | ||||||||||||||
|
|
@@ -24,6 +24,11 @@ | |||||||||||||
| "import": "./dist/esm/browser.js", | ||||||||||||||
| "require": "./dist/cjs/browser.cjs" | ||||||||||||||
| }, | ||||||||||||||
| "./stores": { | ||||||||||||||
| "types": "./dist/esm/stores.d.ts", | ||||||||||||||
| "import": "./dist/esm/stores.js", | ||||||||||||||
| "require": "./dist/cjs/stores.cjs" | ||||||||||||||
| }, | ||||||||||||||
| "./constants/analytics": { | ||||||||||||||
| "types": "./dist/esm/constants/analytics.d.ts", | ||||||||||||||
| "import": "./dist/esm/constants/analytics.js", | ||||||||||||||
|
|
@@ -64,10 +69,11 @@ | |||||||||||||
| "validate:pkg": "node ./scripts/verify-conditions.mjs" | ||||||||||||||
| }, | ||||||||||||||
| "dependencies": { | ||||||||||||||
| "tslib": "^2.6.2" | ||||||||||||||
| "@selfxyz/common": "workspace:^", | ||||||||||||||
| "tslib": "^2.6.2", | ||||||||||||||
| "zustand": "^4.5.2" | ||||||||||||||
| }, | ||||||||||||||
|
Comment on lines
+72
to
75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid bundling zustand inside the SDK to prevent duplicate copies in apps. Bundling Apply this in tsup configs: // packages/mobile-sdk-alpha/tsup.config.ts (both ESM and CJS configs)
- external: ['react', 'react-native', '@selfxyz/common'],
+ external: ['react', 'react-native', '@selfxyz/common', 'zustand'],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ill keep for now
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aaronmgdr Understood! I'll note that you prefer to keep zustand bundled in the SDK for now. 🐰 🧠 Learnings used |
||||||||||||||
| "devDependencies": { | ||||||||||||||
| "@selfxyz/common": "workspace:^", | ||||||||||||||
| "@testing-library/react": "^14.1.2", | ||||||||||||||
| "@types/react": "^18.3.4", | ||||||||||||||
| "@typescript-eslint/eslint-plugin": "^8.0.0", | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // SPDX-FileCopyrightText: 2025 Social Connect Labs, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
| // NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. | ||
|
|
||
| export { useProtocolStore } from './protocolStore'; |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify compiled entry exists for '@selfxyz/mobile-sdk-alpha/stores'
Metro resolves to dist/esm/stores.js. If this file isn’t emitted, bundling will fail.
🏁 Script executed:
Length of output: 112
Missing compiled entry for '@selfxyz/mobile-sdk-alpha/stores' The file
dist/esm/stores.jsdoesn’t exist—ensure your build emits this file (or update the alias to the correct output path).🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm even after building?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 2744
🏁 Script executed:
Length of output: 3421
@aaronmgdr You're absolutely right! After building, the file should definitely exist.
Looking at your configuration, everything is set up correctly:
stores: 'src/stores/index.ts'entry point ✅src/stores/index.tsexists and exportsuseProtocolStore✅tsupwhich should output todist/esm/stores.js✅devDependencies✅The Metro alias pointing to
dist/esm/stores.jsis correct. The sandbox environment I'm running in doesn't have the build tools available, so I couldn't verify by actually building, but based on your tsup configuration, runningnpm run buildin thepackages/mobile-sdk-alphadirectory should indeed create thedist/esm/stores.jsfile that Metro is looking for.The configuration looks solid - once you build the mobile-sdk-alpha package, the alias should resolve correctly!
🧠 Learnings used