feat: strict eslint #148

Benehiko · 2023-09-21T16:30:44Z

Checklist

I have read the contributing guidelines and signed the CLA.
I have referenced an issue containing the design document if my change introduces a new feature.
I have read the security policy.
I confirm that this pull request does not address a security vulnerability.
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
I have added tests that prove my fix is effective or that my feature works.
I have added the necessary documentation within the code base (if appropriate).

Further comments

src/markup-components/component-wrapper.tsx

src/markup-components/express.spec.ts

src/markup-components/express.ts

src/react-components/ory/helpers/error-messages.tsx

src/theme/message.css.ts

.eslintignore

Benehiko · 2023-09-26T09:57:37Z

.eslintrc.cjs

    "plugin:react/jsx-runtime",
    "plugin:storybook/recommended",
+    "plugin:playwright/recommended",


this eslint plugin shows playwright errors and problems

Benehiko · 2023-09-26T09:58:31Z

playwright-ct.config.ts


 /**
 * See https://playwright.dev/docs/test-configuration.
 */
 export default defineConfig({
-  testDir: "./src/react-components",
-  /* The base directory, relative to the config file, for snapshot files created with toMatchSnapshot and toHaveScreenshot. */
-  snapshotDir: "./__snapshots__",


lets snapshots be generated inside the same directory as the test. this makes it easier to see which snapshot belongs to which test.

Benehiko · 2023-09-26T09:59:36Z

src/markup-components/express.spec.ts

this test doesn't run yet. playwright is set to only run tests inside the react folder. I will still need to figure out a way to run this file since vanillaextract needs to be transpiled before the run.

Benehiko · 2023-09-26T10:00:55Z

src/markup/express.ts

this file wasn't used? it's a duplicate of what we have inside the src/markup-components.

jonas-jonas

Nice, just a few comments.

.eslintignore

src/react-components/button-link.tsx

src/react-components/ory/helpers/node.tsx

src/react-components/tests/translations.spec.ts

mszekiel

Looks good, take a look on the assertions again.

Co-authored-by: Jonas Hungershausen <[email protected]>

Benehiko · 2023-09-29T07:49:52Z

src/react-components/ory/helpers/node.tsx

+      if (Array.isArray(value)) {
+        return {
+          ...accumulator,
+          [key]: value,
+          [key + "_list"]: intl.formatList<string>(value),
+        }
+      } else if (key.endsWith("_unix")) {
+        if (typeof value === "number") {
+          return {
+            ...accumulator,
+            [key]: intl.formatDate(new Date(value * 1000)),
+            [key + "_since"]: intl.formatDateTimeRange(
+              new Date(value),
+              new Date(),
+            ),
+            [key + "_since_minutes"]: Math.abs(
+              (value - new Date().getTime() / 1000) / 60,
+            ).toFixed(2),
+            [key + "_until"]: intl.formatDateTimeRange(
+              new Date(),
+              new Date(value),
+            ),
+            [key + "_until_minutes"]: Math.abs(
+              (new Date().getTime() / 1000 - value) / 60,
+            ).toFixed(2),
+          }
+        }
+      }
+      return {
+        ...accumulator,
+        [key]: value as string | number,
+      }
+    },
+    {},
+  )
+
+  return intl.formatMessage(


This needs some review. I am changing the date format as well here to only have 2 decimal places in the minutes we display. For example "the flow has expired 10.00 minutes ago".

Right now it's a bit unwieldy and would display "the flow has expired -10.0004123 minutes ago".

The values are also absolute now. So no more "-10.00".

There are some tests inside the error-flow-nodes.spec file and inside the node.spec.tsx file.

Probably makes more sense to fix this in kratos?

kratos gives back a unix timestamp from where it expired. So the date.
The ui then has to render the time or the minutes from/until that date.

Ah I get it, this basically formats the timestamp in separate keys (until, since, ...). Makes sense to me

yeah exactly. Do you think it's fine to merge?

Benehiko requested a review from jonas-jonas September 21, 2023 16:31

Benehiko mentioned this pull request Sep 21, 2023

fix: registration via code and verification/recovery card stretches #145

Merged

6 tasks

Benehiko requested a review from zepatrik September 21, 2023 16:34