Skip to content

fix: predeployed connector warning #400

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 21, 2025
Merged

fix: predeployed connector warning #400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/early-steaks-perform.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@dojoengine/predeployed-connector": patch
"@dojoengine/core": patch
"template-vite-ts": patch
"@dojoengine/create-burner": patch
"@dojoengine/create-dojo": patch
"@dojoengine/react": patch
"@dojoengine/sdk": patch
"@dojoengine/state": patch
"@dojoengine/torii-client": patch
"@dojoengine/torii-wasm": patch
"@dojoengine/utils": patch
"@dojoengine/utils-wasm": patch
---

fix: predeployed warnings
24 changes: 24 additions & 0 deletions examples/example-predeployed-connector/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Logs
logs
*.log
npm-debug.log*
yarn-debug.log*
yarn-error.log*
pnpm-debug.log*
lerna-debug.log*

node_modules
dist
dist-ssr
*.local

# Editor directories and files
.vscode/*
!.vscode/extensions.json
.idea
.DS_Store
*.suo
*.ntvs*
*.njsproj
*.sln
*.sw?
50 changes: 50 additions & 0 deletions examples/example-predeployed-connector/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# React + TypeScript + Vite

This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules.

Currently, two official plugins are available:

- [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/README.md) uses [Babel](https://babeljs.io/) for Fast Refresh
- [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh

## Expanding the ESLint configuration

If you are developing a production application, we recommend updating the configuration to enable type aware lint rules:

- Configure the top-level `parserOptions` property like this:

```js
export default tseslint.config({
languageOptions: {
// other options...
parserOptions: {
project: ["./tsconfig.node.json", "./tsconfig.app.json"],
tsconfigRootDir: import.meta.dirname,
},
},
});
```

- Replace `tseslint.configs.recommended` to `tseslint.configs.recommendedTypeChecked` or `tseslint.configs.strictTypeChecked`
- Optionally add `...tseslint.configs.stylisticTypeChecked`
- Install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) and update the config:

```js
// eslint.config.js
import react from "eslint-plugin-react";

export default tseslint.config({
// Set the react version
settings: { react: { version: "18.3" } },
plugins: {
// Add the react plugin
react,
},
rules: {
// other rules...
// Enable its recommended rules
...react.configs.recommended.rules,
...react.configs["jsx-runtime"].rules,
},
});
```
7 changes: 7 additions & 0 deletions examples/example-predeployed-connector/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifest_dev.json";
Comment on lines +1 to +3
Copy link

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 the manifest file path and add type safety.

The manifest is imported from a path outside the example directory. Let's verify the path and improve type safety:

Consider adding type information for the manifest import:

-import manifest from "../../worlds/dojo-starter/manifest_dev.json";
+import { type Manifest } from "@dojoengine/core";
+import manifest from "../../worlds/dojo-starter/manifest_dev.json" assert { type: "json" };

🏁 Script executed:

#!/bin/bash
# Check if the manifest file exists
if [ -f "worlds/dojo-starter/manifest_dev.json" ]; then
    echo "Manifest file exists"
    # Check if it's valid JSON
    jq '.' worlds/dojo-starter/manifest_dev.json > /dev/null
    if [ $? -eq 0 ]; then
        echo "Manifest is valid JSON"
    else
        echo "Invalid JSON in manifest"
    fi
else
    echo "Manifest file not found"
fi

Length of output: 107

Attention: Manifest File Missing & Type Safety for JSON Import

It appears that the manifest file at the relative path "../../worlds/dojo-starter/manifest_dev.json" could not be located. Please take the following actions:

  • Verify the File's Existence: Confirm if the manifest file exists at the expected path. If it’s been moved or renamed, update the import accordingly.
  • Improve Type Safety: Once the file is available, update the import statement to add type information: 
    -import manifest from "../../worlds/dojo-starter/manifest_dev.json";
+import { type Manifest } from "@dojoengine/core";
+import manifest from "../../worlds/dojo-starter/manifest_dev.json" assert { type: "json" };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { createDojoConfig } from "@dojoengine/core";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";
import { createDojoConfig } from "@dojoengine/core";
import { type Manifest } from "@dojoengine/core";
import manifest from "../../worlds/dojo-starter/manifest_dev.json" assert { type: "json" };


export const dojoConfig = createDojoConfig({
manifest,
});
28 changes: 28 additions & 0 deletions examples/example-predeployed-connector/eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import js from "@eslint/js";
import globals from "globals";
import reactHooks from "eslint-plugin-react-hooks";
import reactRefresh from "eslint-plugin-react-refresh";
import tseslint from "typescript-eslint";

export default tseslint.config(
{ ignores: ["dist"] },
{
extends: [js.configs.recommended, ...tseslint.configs.recommended],
files: ["**/*.{ts,tsx}"],
languageOptions: {
ecmaVersion: 2020,
globals: globals.browser,
},
plugins: {
"react-hooks": reactHooks,
"react-refresh": reactRefresh,
},
rules: {
...reactHooks.configs.recommended.rules,
"react-refresh/only-export-components": [
"warn",
{ allowConstantExport: true },
],
},
}
);
13 changes: 13 additions & 0 deletions examples/example-predeployed-connector/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite + React + TS</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/main.tsx"></script>
</body>
</html>
34 changes: 34 additions & 0 deletions examples/example-predeployed-connector/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"name": "example-predeployed-connector",
"private": true,
"version": "0.0.0",
"type": "module",
"scripts": {
"dev": "vite",
"build": "tsc -b && vite build",
"lint": "eslint .",
"preview": "vite preview"
},
"dependencies": {
"@dojoengine/core": "workspace:*",
"@dojoengine/predeployed-connector": "workspace:*",
"@dojoengine/sdk": "workspace:*",
"@starknet-react/chains": "catalog:",
"@starknet-react/core": "catalog:",
"react": "catalog:",
"react-dom": "catalog:"
},
"devDependencies": {
"@eslint/js": "^9.20.0",
"@types/react": "catalog:",
"@types/react-dom": "catalog:",
"@vitejs/plugin-react": "^4.3.4",
"eslint": "^9.20.1",
"eslint-plugin-react-hooks": "^5.1.0",
"eslint-plugin-react-refresh": "^0.4.19",
"globals": "^15.15.0",
"typescript": "~5.7.3",
"typescript-eslint": "^8.24.1",
"vite": "^6.1.1"
}
}
1 change: 1 addition & 0 deletions examples/example-predeployed-connector/public/vite.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 42 additions & 0 deletions examples/example-predeployed-connector/src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#root {
max-width: 1280px;
margin: 0 auto;
padding: 2rem;
text-align: center;
}

.logo {
height: 6em;
padding: 1.5em;
will-change: filter;
transition: filter 300ms;
}
.logo:hover {
filter: drop-shadow(0 0 2em #646cffaa);
}
.logo.react:hover {
filter: drop-shadow(0 0 2em #61dafbaa);
}

@keyframes logo-spin {
from {
transform: rotate(0deg);
}
to {
transform: rotate(360deg);
}
}

@media (prefers-reduced-motion: no-preference) {
a:nth-of-type(2) .logo {
animation: logo-spin infinite 20s linear;
}
}

.card {
padding: 2em;
}

.read-the-docs {
color: #888;
}
39 changes: 39 additions & 0 deletions examples/example-predeployed-connector/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { useState } from "react";
import reactLogo from "./assets/react.svg";
import viteLogo from "/vite.svg";
import "./App.css";

function App() {
const [count, setCount] = useState(0);

return (
<>
<div>
<a href="https://vite.dev" target="_blank">
<img src={viteLogo} className="logo" alt="Vite logo" />
</a>
Comment on lines +12 to +14
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add security attributes to external links.

Links with target="_blank" should include rel="noreferrer" to prevent potential security vulnerabilities (reverse tabnabbing).

Apply this fix:

-                <a href="https://vite.dev" target="_blank">
+                <a href="https://vite.dev" target="_blank" rel="noreferrer">
                     <img src={viteLogo} className="logo" alt="Vite logo" />
                 </a>
-                <a href="https://react.dev" target="_blank">
+                <a href="https://react.dev" target="_blank" rel="noreferrer">

Also applies to: 15-21

🧰 Tools
🪛 Biome (1.9.4)

[error] 12-12: Avoid using target="_blank" without rel="noreferrer".

Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.

(lint/a11y/noBlankTarget)

<a href="https://react.dev" target="_blank">
<img
src={reactLogo}
className="logo react"
alt="React logo"
/>
</a>
</div>
<h1>Vite + React</h1>
<div className="card">
<button onClick={() => setCount((count) => count + 1)}>
count is {count}
</button>
Comment on lines +25 to +27
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add explicit button type for accessibility.

Buttons should have an explicit type for better accessibility and to prevent unintended form submissions.

Apply this fix:

-                <button onClick={() => setCount((count) => count + 1)}>
+                <button type="button" onClick={() => setCount((count) => count + 1)}>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button onClick={() => setCount((count) => count + 1)}>
count is {count}
</button>
<button type="button" onClick={() => setCount((count) => count + 1)}>
count is {count}
</button>
🧰 Tools
🪛 Biome (1.9.4)

[error] 25-25: Provide an explicit type prop for the button element.

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

(lint/a11y/useButtonType)

<p>
Edit <code>src/App.tsx</code> and save to test HMR
</p>
</div>
<p className="read-the-docs">
Click on the Vite and React logos to learn more
</p>
</>
);
}

export default App;
1 change: 1 addition & 0 deletions examples/example-predeployed-connector/src/assets/react.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
68 changes: 68 additions & 0 deletions examples/example-predeployed-connector/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
:root {
font-family: Inter, system-ui, Avenir, Helvetica, Arial, sans-serif;
line-height: 1.5;
font-weight: 400;

color-scheme: light dark;
color: rgba(255, 255, 255, 0.87);
background-color: #242424;

font-synthesis: none;
text-rendering: optimizeLegibility;
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
}

a {
font-weight: 500;
color: #646cff;
text-decoration: inherit;
}
a:hover {
color: #535bf2;
}

body {
margin: 0;
display: flex;
place-items: center;
min-width: 320px;
min-height: 100vh;
}
Comment on lines +25 to +31
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Flex Container Centering Issue in body
The body is styled as a flex container; however, using place-items: center is ineffective on flex containers—it is intended for CSS grid layouts. To correctly center content both horizontally and vertically, please replace it with:

-    place-items: center;
+    justify-content: center;
+    align-items: center;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
body {
margin: 0;
display: flex;
place-items: center;
min-width: 320px;
min-height: 100vh;
}
body {
margin: 0;
display: flex;
justify-content: center;
align-items: center;
min-width: 320px;
min-height: 100vh;
}


h1 {
font-size: 3.2em;
line-height: 1.1;
}

button {
border-radius: 8px;
border: 1px solid transparent;
padding: 0.6em 1.2em;
font-size: 1em;
font-weight: 500;
font-family: inherit;
background-color: #1a1a1a;
cursor: pointer;
transition: border-color 0.25s;
}
button:hover {
border-color: #646cff;
}
button:focus,
button:focus-visible {
outline: 4px auto -webkit-focus-ring-color;
}

@media (prefers-color-scheme: light) {
:root {
color: #213547;
background-color: #ffffff;
}
a:hover {
color: #747bff;
}
button {
background-color: #f9f9f9;
}
}
13 changes: 13 additions & 0 deletions examples/example-predeployed-connector/src/main.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { StrictMode } from "react";
import { createRoot } from "react-dom/client";
import "./index.css";
import App from "./App.tsx";
import StarknetProvider from "./starknet-provider.tsx";

createRoot(document.getElementById("root")!).render(
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Handle potential null case for root element.

The non-null assertion (!) assumes the root element always exists, which could lead to runtime errors if the element is not found.

Consider this safer approach:

-createRoot(document.getElementById("root")!).render(
+const rootElement = document.getElementById("root");
+if (!rootElement) throw new Error("Failed to find root element");
+createRoot(rootElement).render(
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
createRoot(document.getElementById("root")!).render(
const rootElement = document.getElementById("root");
if (!rootElement) throw new Error("Failed to find root element");
createRoot(rootElement).render(
🧰 Tools
🪛 Biome (1.9.4)

[error] 7-7: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

<StrictMode>
<StarknetProvider>
<App />
</StarknetProvider>
</StrictMode>
);
41 changes: 41 additions & 0 deletions examples/example-predeployed-connector/src/starknet-provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { useEffect, useState, type PropsWithChildren } from "react";
import { mainnet } from "@starknet-react/chains";
import { jsonRpcProvider, StarknetConfig, voyager } from "@starknet-react/core";
import { dojoConfig } from "../dojoConfig";
import {
predeployedAccounts,
type PredeployedAccountsConnector,
} from "@dojoengine/predeployed-connector";

export default function StarknetProvider({ children }: PropsWithChildren) {
const [retries, setRetries] = useState<number>(0);
const [connectors, setConnectors] = useState<
PredeployedAccountsConnector[]
>([]);
const provider = jsonRpcProvider({
rpc: () => ({ nodeUrl: dojoConfig.rpcUrl as string }),
});

useEffect(() => {
if (connectors.length === 0 && retries < 5) {
setRetries(retries + 1);
predeployedAccounts({
rpc: dojoConfig.rpcUrl as string,
id: "katana",
name: "Katana",
}).then(setConnectors);
}
}, [connectors]);
Comment on lines +19 to +28
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix missing dependency in useEffect hook.

The retries state is used inside the effect but not included in the dependency array, which could lead to stale closures.

Apply this fix:

-    }, [connectors]);
+    }, [connectors, retries]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (connectors.length === 0 && retries < 5) {
setRetries(retries + 1);
predeployedAccounts({
rpc: dojoConfig.rpcUrl as string,
id: "katana",
name: "Katana",
}).then(setConnectors);
}
}, [connectors]);
useEffect(() => {
if (connectors.length === 0 && retries < 5) {
setRetries(retries + 1);
predeployedAccounts({
rpc: dojoConfig.rpcUrl as string,
id: "katana",
name: "Katana",
}).then(setConnectors);
}
}, [connectors, retries]);
🧰 Tools
🪛 Biome (1.9.4)

[error] 19-19: This hook does not specify all of its dependencies: retries

This dependency is not specified in the hook dependency list.

This dependency is not specified in the hook dependency list.

(lint/correctness/useExhaustiveDependencies)


return (
<StarknetConfig
chains={[mainnet]}
provider={provider}
connectors={connectors}
explorer={voyager}
autoConnect
>
{children}
</StarknetConfig>
);
}
Loading
Loading