feat: enable verbatim module on @ory/elements-react and update imports#599
Open
GauthierPLM wants to merge 2 commits intoory:mainfrom
Open
feat: enable verbatim module on @ory/elements-react and update imports#599GauthierPLM wants to merge 2 commits intoory:mainfrom
@ory/elements-react and update imports#599GauthierPLM wants to merge 2 commits intoory:mainfrom
Conversation
|
@GauthierPLM is attempting to deploy a commit to the ory Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (150)
✅ Files skipped from review due to trivial changes (144)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR converts many runtime value imports to TypeScript type-only imports across the elements-react package and adds ChangesType-Only Import Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
GauthierPLM
force-pushed
the
refactor/verbatim-module-on-elements-react
branch
from
6c0661c to
52914a2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/elements-react/src/components/form/nodes/node.tsx (1)
56-56:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix missing
Reactnamespace import on line 56.Line 56 uses
React.HTMLAttributeReferrerPolicyin a type cast, butReactis not imported as a namespace. The file only importsReactNodeas a type-only import (line 5), which does not make theReactnamespace available. WithverbatimModuleSyntax: trueandjsx: react-jsxin tsconfig, TypeScript will fail to resolveReactat compile time.Add a namespace import:
+import type * as React from "react" import type { ReactNode } from "react"Or use inline import syntax in the cast:
-referrerPolicy={referrerpolicy as React.HTMLAttributeReferrerPolicy} +referrerPolicy={referrerpolicy as import("react").HTMLAttributeReferrerPolicy}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/elements-react/src/components/form/nodes/node.tsx` at line 56, The cast using React.HTMLAttributeReferrerPolicy in the referrerPolicy prop is failing because the React namespace is not imported (only the type ReactNode is imported); fix by bringing the React namespace into scope or by using an inline import type cast: either add a namespace import for React (e.g., import * as React from 'react') so React.HTMLAttributeReferrerPolicy resolves, or change the cast to the inline form (import('react').HTMLAttributeReferrerPolicy) so the type is resolved without a namespace; update the referrerPolicy usage accordingly near the referrerpolicy prop and ensure imports at the top include the chosen change.
🧹 Nitpick comments (3)
packages/elements-react/stories/utils.ts (1)
12-12: ⚡ Quick winAvoid importing from internal
@ory/client-fetch/src/paths.The
LoginFlowActiveEnumis not part of the@ory/client-fetchpublic API surface. Importing directly fromsrc/bypasses the public interface and can silently break on internal package refactors. Since the enum is not publicly exported, inline the type as a string union literal instead:♻️ Suggested fix
-import { LoginFlowActiveEnum } from "@ory/client-fetch/src/models/LoginFlow" +type LoginFlowActiveMethod = "password" | "oidc" | "totp" | "webauthn" | "lookup_secret" | "code"Then replace
LoginFlowActiveEnumusages with the local type.Note: This pattern exists in multiple story files and should be addressed across all of them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/elements-react/stories/utils.ts` at line 12, Remove the non-public import of LoginFlowActiveEnum (import { LoginFlowActiveEnum } ...) and replace it with a local string-union type (e.g., type LoginFlowActive = 'active' | 'pending' | ...) declared in this file, then update all usages of LoginFlowActiveEnum in this file (and other story files with the same pattern) to use the local type name (e.g., LoginFlowActive) so you no longer import from `@ory/client-fetch/src/`.packages/elements-react/src/context/defaultNodeSorter.test.ts (1)
5-5: ⚡ Quick win
UiNodeInputAttributesTypeEnumis only used as a type — add thetypemodifier for consistency
UiNodeInputAttributesTypeEnumappears in exactly one position in this file: the parameter type annotation on line 11 (inputType?: UiNodeInputAttributesTypeEnum). It is never used as a runtime value (no member access like.Text,.Submit, etc.; callers always pass string literals). Without thetypemodifier, any import without a type modifier is left in the emitted output; anything that uses the type modifier is dropped entirely. So the import is kept as a runtime reference unnecessarily.Note: TypeScript does not error on a regular enum imported and used only as a type even with
verbatimModuleSyntaxenabled — the missing TS1484 diagnostic is a known compiler gap, so this won't break the build. However, the documented best practice is to useimport typefor enums when used only as types.♻️ Proposed fix
-import { type UiNode, UiNodeInputAttributesTypeEnum } from "@ory/client-fetch" +import { type UiNode, type UiNodeInputAttributesTypeEnum } from "@ory/client-fetch"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/elements-react/src/context/defaultNodeSorter.test.ts` at line 5, Change the import so UiNodeInputAttributesTypeEnum is imported only as a type (e.g. add the type modifier for UiNodeInputAttributesTypeEnum) because it’s used only in the parameter annotation inputType?: UiNodeInputAttributesTypeEnum; update the import line that currently mentions UiNode and UiNodeInputAttributesTypeEnum so that UiNodeInputAttributesTypeEnum is prefixed with type (or moved to an import type {...} statement) to avoid emitting an unnecessary runtime reference.packages/elements-react/src/util/events.ts (1)
4-17: ⚡ Quick win
FlowTypeis never used as a value here — consider marking ittype FlowType.Every reference to
FlowTypein this file (lines 32, 44, 61, 75, 86, 97, 134–138, 153–163) is inside anexport type { ... }declaration — none are runtime value accesses. WithverbatimModuleSyntax: true, a non-typeimport is emitted verbatim into JavaScript, so the compiledevents.jswill carry an unnecessary runtime import ofFlowTypefrom@ory/client-fetch.TypeScript will not raise a compile error here (regular enum imports are intentionally not flagged even when only used as types), but the intent of this PR is to make module-level type dependencies explicit and avoid opaque runtime coupling.
♻️ Suggested fix
import { type ErrorFlowReplaced, - FlowType, + type FlowType, type GenericError,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/elements-react/src/util/events.ts` around lines 4 - 17, The import includes FlowType as a value but it's only used in type-only exports (e.g., the export type { FlowType, ... } occurrences), so change the import specifier for FlowType to a type-only import (i.e., import type { FlowType } from "@ory/client-fetch") while leaving the other runtime imports unchanged; update the import statement that currently lists FlowType alongside ErrorFlowReplaced, GenericError, Identity, LoginFlow, etc., to import FlowType with the type keyword so the compiled events.ts does not emit an unnecessary runtime import.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/elements-react/src/context/config.tsx`:
- Line 1: The committed file packages/elements-react/src/context/config.tsx is
not formatted according to the project's formatter; re-run the project's
formatter (e.g., Prettier) on that file, stage and commit the formatted changes
so `git diff HEAD --exit-code` passes; ensure you run the same format command
used in CI (or npm/yarn script like "format") and verify formatting for any
exported symbols in this file (e.g., ConfigContext, ConfigProvider) before
pushing.
- Line 4: The import of FrontendApi in
packages/elements-react/src/context/config.tsx is used only as a type and must
be imported as a type-only import to satisfy verbatimModuleSyntax; update the
import statement that currently references FrontendApi to use the type modifier
(so FrontendApi is imported as a type) so the type alias frontend: FrontendApi
compiles without TS1484 errors.
In
`@packages/elements-react/src/theme/default/components/card/auth-method-list-item.tsx`:
- Line 10: The file fails CI formatting due to an import line (import { type
OryCardAuthMethodListItemProps, useOryFlow } from "@ory/elements-react") — run
the project formatter (e.g., prettier --write or the repo's format script) on
packages/elements-react/src/theme/default/components/card/auth-method-list-item.tsx
to reflow the long import; ensure the import statement for
OryCardAuthMethodListItemProps and useOryFlow matches the repo’s Prettier
configuration (line breaks/spacing) so git diff HEAD --exit-code is clean.
In `@packages/elements-react/src/theme/default/utils/constructCardHeader.ts`:
- Around line 5-12: The import for AuthenticatorAssuranceLevel is missing the
type qualifier causing TS errors with verbatimModuleSyntax; update the import
list in constructCardHeader.ts to import AuthenticatorAssuranceLevel with the
type modifier (i.e., add "type" before AuthenticatorAssuranceLevel) so it
matches the pattern used for OAuth2ConsentRequest, Session, and UiContainer and
satisfies the usage at the requested_aal?: AuthenticatorAssuranceLevel type
position.
In `@packages/elements-react/src/util/i18n/index.ts`:
- Line 5: Import useIntl as a type-only import because it's only used in a type
position (ReturnType<typeof useIntl>) — change the import statement to use the
TypeScript "type" specifier for useIntl (i.e., import { defineMessages, type
IntlShape, type useIntl } from "react-intl") so the binding is treated as
type-only and satisfies verbatimModuleSyntax; update the import that currently
includes useIntl without "type" and ensure ReturnType<typeof useIntl> remains
unchanged.
---
Outside diff comments:
In `@packages/elements-react/src/components/form/nodes/node.tsx`:
- Line 56: The cast using React.HTMLAttributeReferrerPolicy in the
referrerPolicy prop is failing because the React namespace is not imported (only
the type ReactNode is imported); fix by bringing the React namespace into scope
or by using an inline import type cast: either add a namespace import for React
(e.g., import * as React from 'react') so React.HTMLAttributeReferrerPolicy
resolves, or change the cast to the inline form
(import('react').HTMLAttributeReferrerPolicy) so the type is resolved without a
namespace; update the referrerPolicy usage accordingly near the referrerpolicy
prop and ensure imports at the top include the chosen change.
---
Nitpick comments:
In `@packages/elements-react/src/context/defaultNodeSorter.test.ts`:
- Line 5: Change the import so UiNodeInputAttributesTypeEnum is imported only as
a type (e.g. add the type modifier for UiNodeInputAttributesTypeEnum) because
it’s used only in the parameter annotation inputType?:
UiNodeInputAttributesTypeEnum; update the import line that currently mentions
UiNode and UiNodeInputAttributesTypeEnum so that UiNodeInputAttributesTypeEnum
is prefixed with type (or moved to an import type {...} statement) to avoid
emitting an unnecessary runtime reference.
In `@packages/elements-react/src/util/events.ts`:
- Around line 4-17: The import includes FlowType as a value but it's only used
in type-only exports (e.g., the export type { FlowType, ... } occurrences), so
change the import specifier for FlowType to a type-only import (i.e., import
type { FlowType } from "@ory/client-fetch") while leaving the other runtime
imports unchanged; update the import statement that currently lists FlowType
alongside ErrorFlowReplaced, GenericError, Identity, LoginFlow, etc., to import
FlowType with the type keyword so the compiled events.ts does not emit an
unnecessary runtime import.
In `@packages/elements-react/stories/utils.ts`:
- Line 12: Remove the non-public import of LoginFlowActiveEnum (import {
LoginFlowActiveEnum } ...) and replace it with a local string-union type (e.g.,
type LoginFlowActive = 'active' | 'pending' | ...) declared in this file, then
update all usages of LoginFlowActiveEnum in this file (and other story files
with the same pattern) to use the local type name (e.g., LoginFlowActive) so you
no longer import from `@ory/client-fetch/src/`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eb1bb19f-aef1-4a30-bc2b-1bdb7cb5576d
📒 Files selected for processing (150)
packages/elements-react/src/client/frontendClient.tspackages/elements-react/src/client/session-provider.tsxpackages/elements-react/src/client/useSession.spec.tsxpackages/elements-react/src/components/card/card.tsxpackages/elements-react/src/components/card/content.tsxpackages/elements-react/src/components/card/index.tsxpackages/elements-react/src/components/card/two-step/__tests__/card-two-step.spec.tspackages/elements-react/src/components/card/two-step/state-method-active.tsxpackages/elements-react/src/components/card/two-step/state-provide-identifier.tsxpackages/elements-react/src/components/card/two-step/state-select-method.tsxpackages/elements-react/src/components/card/two-step/utils.tspackages/elements-react/src/components/form/form-helpers.tspackages/elements-react/src/components/form/form-provider.tsxpackages/elements-react/src/components/form/form-resolver.test.tsxpackages/elements-react/src/components/form/form-resolver.tspackages/elements-react/src/components/form/form.tsxpackages/elements-react/src/components/form/messages.tsxpackages/elements-react/src/components/form/nodes/hooks/useInputProps.tsxpackages/elements-react/src/components/form/nodes/input.tsxpackages/elements-react/src/components/form/nodes/node-button.tsxpackages/elements-react/src/components/form/nodes/node.tsxpackages/elements-react/src/components/form/nodes/renderer/button-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/checkbox-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/consent-checkbox-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/hidden-input-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/image-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/input-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/select-renderer.test.tsxpackages/elements-react/src/components/form/nodes/renderer/select-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/sso-button-renderer.tsxpackages/elements-react/src/components/form/nodes/renderer/text-renderer.tsxpackages/elements-react/src/components/form/settings-section.tsxpackages/elements-react/src/components/form/social.test.tsxpackages/elements-react/src/components/form/social.tsxpackages/elements-react/src/components/form/useOryFormSubmit.tspackages/elements-react/src/components/form/useResendCode.tspackages/elements-react/src/components/settings/index.tsxpackages/elements-react/src/components/settings/oidc-settings.tsxpackages/elements-react/src/components/settings/passkey-settings.tsxpackages/elements-react/src/components/settings/recovery-codes-settings.tsxpackages/elements-react/src/components/settings/settings-card.tsxpackages/elements-react/src/components/settings/totp-settings.tsxpackages/elements-react/src/components/settings/webauthn-settings.tsxpackages/elements-react/src/context/component.tsxpackages/elements-react/src/context/config.tsxpackages/elements-react/src/context/defaultNodeSorter.test.tspackages/elements-react/src/context/defaultNodeSorter.tspackages/elements-react/src/context/flow-context.tsxpackages/elements-react/src/context/form-state.test.tspackages/elements-react/src/context/form-state.tspackages/elements-react/src/context/intl-context.tsxpackages/elements-react/src/context/provider.tsxpackages/elements-react/src/tests/jest/test-utils.tsxpackages/elements-react/src/theme/default/components/card/auth-method-list-container.tsxpackages/elements-react/src/theme/default/components/card/auth-method-list-item.tsxpackages/elements-react/src/theme/default/components/card/content.tsxpackages/elements-react/src/theme/default/components/card/current-identifier-button.test.tspackages/elements-react/src/theme/default/components/card/current-identifier-button.tsxpackages/elements-react/src/theme/default/components/card/footer.test.tsxpackages/elements-react/src/theme/default/components/card/footer.tsxpackages/elements-react/src/theme/default/components/card/index.tsxpackages/elements-react/src/theme/default/components/card/list-item.tsxpackages/elements-react/src/theme/default/components/default-components.tsxpackages/elements-react/src/theme/default/components/form/button.tsxpackages/elements-react/src/theme/default/components/form/captcha.tsxpackages/elements-react/src/theme/default/components/form/checkbox.tsxpackages/elements-react/src/theme/default/components/form/consent-scope-checkbox.tsxpackages/elements-react/src/theme/default/components/form/group-container.tsxpackages/elements-react/src/theme/default/components/form/image.tsxpackages/elements-react/src/theme/default/components/form/index.tsxpackages/elements-react/src/theme/default/components/form/input.tsxpackages/elements-react/src/theme/default/components/form/label.tsxpackages/elements-react/src/theme/default/components/form/link-button.tsxpackages/elements-react/src/theme/default/components/form/pin-code-input.tsxpackages/elements-react/src/theme/default/components/form/section.tsxpackages/elements-react/src/theme/default/components/form/select.tsxpackages/elements-react/src/theme/default/components/form/sso.tsxpackages/elements-react/src/theme/default/components/form/text.tsxpackages/elements-react/src/theme/default/components/generic/page-header.tsxpackages/elements-react/src/theme/default/components/generic/toast.tsxpackages/elements-react/src/theme/default/components/settings/settings-oidc.tsxpackages/elements-react/src/theme/default/components/settings/settings-passkey.tsxpackages/elements-react/src/theme/default/components/settings/settings-recovery-codes.tsxpackages/elements-react/src/theme/default/components/settings/settings-totp.tsxpackages/elements-react/src/theme/default/components/settings/settings-webauthn.tsxpackages/elements-react/src/theme/default/components/ui/checkbox-label.spec.tsxpackages/elements-react/src/theme/default/components/ui/checkbox-label.tsxpackages/elements-react/src/theme/default/components/ui/dropdown-menu.tsxpackages/elements-react/src/theme/default/components/ui/user-avater.tsxpackages/elements-react/src/theme/default/components/ui/user-menu.tsxpackages/elements-react/src/theme/default/flows/consent.tsxpackages/elements-react/src/theme/default/flows/error.tsxpackages/elements-react/src/theme/default/flows/login.tsxpackages/elements-react/src/theme/default/flows/recovery.tsxpackages/elements-react/src/theme/default/flows/registration.tsxpackages/elements-react/src/theme/default/flows/settings.tsxpackages/elements-react/src/theme/default/flows/verification.tsxpackages/elements-react/src/theme/default/utils/__tests__/constructCardHeader.spec.tsxpackages/elements-react/src/theme/default/utils/__tests__/user.spec.tspackages/elements-react/src/theme/default/utils/constructCardHeader.tspackages/elements-react/src/theme/default/utils/logout.tspackages/elements-react/src/theme/default/utils/oauth2.tspackages/elements-react/src/theme/default/utils/user.tspackages/elements-react/src/types.tspackages/elements-react/src/util/childCounter.tspackages/elements-react/src/util/client.tspackages/elements-react/src/util/clientConfiguration.tspackages/elements-react/src/util/events.tspackages/elements-react/src/util/flowContainer.tspackages/elements-react/src/util/flowHasErrors.tspackages/elements-react/src/util/i18n/__tests__/index.spec.tsxpackages/elements-react/src/util/i18n/__tests__/test-components.tsxpackages/elements-react/src/util/i18n/index.tspackages/elements-react/src/util/nodes.tspackages/elements-react/src/util/omitAttributes.tspackages/elements-react/src/util/onSubmitLogin.test.tspackages/elements-react/src/util/onSubmitLogin.tspackages/elements-react/src/util/onSubmitRecovery.test.tspackages/elements-react/src/util/onSubmitRecovery.tspackages/elements-react/src/util/onSubmitRegistration.test.tspackages/elements-react/src/util/onSubmitRegistration.tspackages/elements-react/src/util/onSubmitSettings.test.tspackages/elements-react/src/util/onSubmitSettings.tspackages/elements-react/src/util/onSubmitVerification.test.tspackages/elements-react/src/util/onSubmitVerification.tspackages/elements-react/src/util/sdk-helpers/__test__/continueWith.spec.tspackages/elements-react/src/util/sdk-helpers/__test__/ui.spec.tspackages/elements-react/src/util/sdk-helpers/__test__/utils.spec.tspackages/elements-react/src/util/sdk-helpers/continueWith.tspackages/elements-react/src/util/sdk-helpers/error.tspackages/elements-react/src/util/sdk-helpers/ui.tspackages/elements-react/src/util/sdk-helpers/utils.tspackages/elements-react/src/util/showToast.tsxpackages/elements-react/src/util/submitHandler.tspackages/elements-react/src/util/transientPayload.test.tspackages/elements-react/src/util/transientPayload.tspackages/elements-react/src/util/ui/__test__/ui.spec.tspackages/elements-react/src/util/ui/index.tspackages/elements-react/src/util/utilFixSDKTypesHelper.tspackages/elements-react/stories/components/error/error.stories.tsxpackages/elements-react/stories/components/settings/all-2fa.stories.tspackages/elements-react/stories/components/settings/all.stories.tsxpackages/elements-react/stories/components/settings/lookup_secrets.stories.tspackages/elements-react/stories/components/settings/oidc.stories.tspackages/elements-react/stories/components/settings/passkey.stories.tspackages/elements-react/stories/components/settings/saml.stories.tspackages/elements-react/stories/components/settings/totp.stories.tspackages/elements-react/stories/components/settings/webauthn.stories.tspackages/elements-react/stories/utils.tspackages/elements-react/tsconfig.json
Author
|
Not sure to understand the issue with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enables
verbatimModuleSyntaxin the TypeScript compiler options and adds the type qualifier to every type-only import across the codebase (e.g.import type { Foo } from '...').Without verbatimModuleSyntax, TypeScript silently erases type-only imports at compile time in a way that is opaque to bundlers and module-aware tools. I encountered such issues when importing the stories in my project to have a preview of our custom styling: some components / imports used
typebut a large number of them did not.Instead of only fixing it in stories on our side, I thought I could spend a bit more time to contribute these changes.
Related Issue or Design Document
Checklist
and signed the CLA.
introduces a new feature.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got approval (please contact
security@ory.com) from the maintainers to push
the changes.
works.
appropriate).
Further comments
I know that the contributing guidelines require to open a discussion / issue prior to opening a PR, but I read them after finishing the changes.
Let me know if the PR isn't fit and I will close it.
Summary by CodeRabbit