Skip to content

Require project environment in SDK #14679

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 22 commits into from
Nov 20, 2024
Merged

Require project environment in SDK #14679

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
186822e
Require project environment in SDK
bzwrk Nov 18, 2024
d34e259
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 18, 2024
647e18c
Refactor environment check into its own method
bzwrk Nov 18, 2024
d91741b
Define a new type for project environment
bzwrk Nov 19, 2024
c1781ed
Fix comment and update CHANGELOG.md
bzwrk Nov 19, 2024
40bab31
Fix linter errors
bzwrk Nov 19, 2024
7753a5b
Fix linter errors
bzwrk Nov 19, 2024
a904e53
Update package lock
bzwrk Nov 19, 2024
5efcc32
Update gitignore
bzwrk Nov 19, 2024
45b4875
Add eslint config
bzwrk Nov 19, 2024
7abedce
Apply eslint
bzwrk Nov 19, 2024
8de3f44
Apply linter suggestions
bzwrk Nov 19, 2024
16704ee
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 19, 2024
9d5e709
Fix markdown syntax errors
bzwrk Nov 19, 2024
d0362d0
Fix tests
bzwrk Nov 19, 2024
2070190
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 19, 2024
87fb597
Update pnpm lock
bzwrk Nov 19, 2024
5b86f27
Remove pnpm lock changes
bzwrk Nov 19, 2024
041a594
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 20, 2024
a171866
Remove eslint configs
bzwrk Nov 20, 2024
96fc56b
Undo eslint change
bzwrk Nov 20, 2024
044daed
Revert reversion of eslint changes
bzwrk Nov 20, 2024
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ docs/.vuepress/dist

./package-lock.json
components/**/package-lock.json
/packages/evals/
/packages/sdk/examples/.next/
27 changes: 27 additions & 0 deletions eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import globals from "globals";
import pluginJs from "@eslint/js";
import tseslint from "typescript-eslint";

/** @type {import('eslint').Linter.Config[]} */
export default [
{
files: [
"**/*.{js,mjs,cjs,ts}",
],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Empty configuration object detected.

The first configuration object specifies file patterns but doesn't include any actual configuration. This makes the object redundant.

Consider removing this empty configuration or adding necessary rules:

- {
-   files: [
-     "**/*.{js,mjs,cjs,ts}",
-   ],
- },
📝 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
{
files: [
"**/*.{js,mjs,cjs,ts}",
],
},

{
files: [
"**/*.js",
],
languageOptions: {
sourceType: "commonjs",
},
},
{
languageOptions: {
globals: globals.browser,
},
},
pluginJs.configs.recommended,
...tseslint.configs.recommended,
];
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance TypeScript configuration for better type safety.

Given that this PR involves TypeScript code and type safety (particularly for the ProjectEnvironment type), the ESLint configuration should be strengthened for TypeScript files.

Consider adding these TypeScript-specific configurations:

 export default [
-  {
-    files: [
-      "**/*.{js,mjs,cjs,ts}",
-    ],
-  },
+  {
+    files: ["**/*.ts"],
+    languageOptions: {
+      parser: tseslint.parser,
+      parserOptions: {
+        project: "./tsconfig.json",
+        ecmaVersion: "latest",
+        sourceType: "module"
+      }
+    },
+    plugins: {
+      "@typescript-eslint": tseslint.plugin
+    },
+    rules: {
+      "@typescript-eslint/strict-boolean-expressions": "error",
+      "@typescript-eslint/no-explicit-any": "error",
+      "@typescript-eslint/explicit-function-return-type": "error",
+      "@typescript-eslint/no-unused-vars": "error"
+    }
+  },
   {
     files: [
       "**/*.js",
     ],
     languageOptions: {
       sourceType: "commonjs",
     },
   },
   {
     languageOptions: {
       globals: globals.browser,
     },
   },
   pluginJs.configs.recommended,
   ...tseslint.configs.recommended,
 ];

These rules will help enforce:

  • Proper type checking in boolean expressions
  • No usage of any type
  • Explicit return types for functions
  • No unused variables

This aligns with the PR's focus on type safety for the ProjectEnvironment implementation.

Committable suggestion skipped: line range outside the PR's diff.

Loading
Loading