additive react-mentions-ts/core entry while keeping the root entry st…#194
additive react-mentions-ts/core entry while keeping the root entry st…#194hbmartin wants to merge 1 commit into
Conversation
…yled/backward-compatible. The split is now explicit: core uses dependency-free structural styles and class concatenation, while styled root keeps cva, clsx, and tailwind-merge in src/styles/styled.ts (line 42). Updated packaging/build exports in package.json (line 11) and tsdown.config.ts (line 4), added core entry tests in src/coreEntry.spec.tsx (line 1), and removed the stale src/utils/cn.ts helper.
📝 WalkthroughWalkthroughThis PR refactors the mentions input library to separate styled and unstyled implementations. It extracts component logic into reusable base classes, introduces a centralized styling system with optional Tailwind support, adds a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'core' entrypoint that provides a dependency-free version of the library, removing the requirement for Tailwind CSS, class-variance-authority, and tailwind-merge. The implementation refactors existing components into base versions that accept a style configuration object, enabling both the default styled entry and the new core entry to share logic. A review comment suggests improving the mergeStyles utility by ensuring it always returns a shallow copy of style objects to prevent accidental mutation of the source configuration.
| continue | ||
| } | ||
|
|
||
| merged = merged === undefined ? style : { ...merged, ...style } |
There was a problem hiding this comment.
Assigning merged = style directly creates a reference to the input object. If the resulting merged style object is later mutated (though unlikely in standard React usage), it could inadvertently modify the source style object from the configuration. It is safer to always create a shallow copy.
| merged = merged === undefined ? style : { ...merged, ...style } | |
| merged = merged === undefined ? { ...style } : { ...merged, ...style } |
There was a problem hiding this comment.
Pull request overview
This PR introduces an additive react-mentions-ts/core entrypoint that avoids styled/Tailwind helper dependencies, while keeping the existing root entry styled and backward-compatible via an explicit split between core structural styles and styled Tailwind defaults.
Changes:
- Add a new
corebuild entry + package export (react-mentions-ts/core) and update packaging metadata accordingly. - Refactor styling/rendering into “Base” components and
MentionsInputBase, with style behavior injected viasrc/styles/core.ts(dependency-free) orsrc/styles/styled.ts(Tailwind/CVA). - Add tests validating the core entry renders without Tailwind defaults and does not pull styled-only dependencies into the core source graph; remove the stale
cnhelper.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tsdown.config.ts | Adds a second build entry for core alongside index. |
| src/utils/index.ts | Removes the cn re-export from the utils barrel. |
| src/utils/cn.ts | Deletes the Tailwind/clsx-based cn helper. |
| src/styles/types.ts | Introduces shared style-config types (class merge + inline style hooks). |
| src/styles/tailwindMergeClassNames.ts | Adds Tailwind-aware class merger used by the styled entry. |
| src/styles/styled.ts | Defines the styled/Tailwind defaults style config (uses CVA + twMerge). |
| src/styles/mergeStyles.ts | Adds a small helper to merge optional inline style objects. |
| src/styles/joinClassNames.ts | Adds dependency-free class concatenation used by core. |
| src/styles/core.ts | Defines dependency-free “structural” inline styles and empty class defaults for core. |
| src/coreEntry.spec.tsx | Adds tests for the new core entry behavior and dependency isolation. |
| src/core/MentionsInput.tsx | Adds a core MentionsInput that uses MentionsInputBase + core styles. |
| src/core/Mention.tsx | Adds a core Mention wrapper using MentionBase + core styles. |
| src/core.ts | Adds the public react-mentions-ts/core entry exports (components/types/helpers). |
| src/SuggestionsOverlayBase.tsx | Extracts overlay logic into a style-injected base component. |
| src/SuggestionsOverlay.tsx | Converts to a thin styled wrapper over SuggestionsOverlayBase. |
| src/SuggestionBase.tsx | Extracts suggestion rendering into a style-injected base component. |
| src/Suggestion.tsx | Converts to a thin styled wrapper over SuggestionBase. |
| src/MentionsInputView.tsx | Adds controlStyle support so core can provide structural positioning via inline styles. |
| src/MentionsInputSelectors.ts | Switches to direct utility imports (avoids barrel coupling). |
| src/MentionsInputQueryState.ts | Switches to direct utility imports. |
| src/MentionsInputEditing.ts | Switches to direct utility imports. |
| src/MentionsInputDerived.ts | Switches to direct utility imports. |
| src/MentionsInputBase.tsx | Introduces MentionsInputBase that drives behavior and consumes an injected style config. |
| src/MentionsInput.tsx | Converts to a thin styled wrapper class over MentionsInputBase. |
| src/MentionsInput.spec.tsx | Updates spies/imports to reflect direct utility imports and new module structure. |
| src/MentionsInput.performance.spec.tsx | Updates perf spies and thresholds to match refactor behavior. |
| src/MentionBase.tsx | Extracts mention rendering into a style-injected base component. |
| src/Mention.tsx | Converts to a thin styled wrapper over MentionBase. |
| src/LoadingIndicatorBase.tsx | Extracts loading indicator into a style-injected base component. |
| src/LoadingIndicator.tsx | Converts to a thin styled wrapper over LoadingIndicatorBase. |
| src/HighlighterBase.tsx | Extracts highlighter logic into a style-injected base component. |
| src/Highlighter.tsx | Converts to a thin styled wrapper over HighlighterBase. |
| package.json | Adds ./core export + typesVersions mapping, adjusts sideEffects, and makes styling peers optional. |
| README.md | Documents styled vs core entry usage and Tailwind helper expectations. |
| .attw.json | Excludes styles/tailwind.css from arethetypeswrong entrypoint checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 35 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/HighlighterBase.tsx">
<violation number="1" location="src/HighlighterBase.tsx:220">
P2: Caret marker is not rendered when the caret is at the end of a value that ends with a mention, causing missing/stale caret position updates.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:175">
P2: Do not mark `class-variance-authority`, `clsx`, and `tailwind-merge` as optional peers while the default root export still requires them.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // append a span containing a space, to ensure the last text line has the correct height | ||
| components.push(' ') | ||
|
|
||
| if (components !== resultComponents) { |
There was a problem hiding this comment.
P2: Caret marker is not rendered when the caret is at the end of a value that ends with a mention, causing missing/stale caret position updates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/HighlighterBase.tsx, line 220:
<comment>Caret marker is not rendered when the caret is at the end of a value that ends with a mention, causing missing/stale caret position updates.</comment>
<file context>
@@ -0,0 +1,253 @@
+ // append a span containing a space, to ensure the last text line has the correct height
+ components.push(' ')
+
+ if (components !== resultComponents) {
+ resultComponents.push(renderCaretMarker(), ...components)
+ }
</file context>
| "react-dom": ">=19.0.0", | ||
| "tailwind-merge": ">=3.0.0" | ||
| }, | ||
| "peerDependenciesMeta": { |
There was a problem hiding this comment.
P2: Do not mark class-variance-authority, clsx, and tailwind-merge as optional peers while the default root export still requires them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 175:
<comment>Do not mark `class-variance-authority`, `clsx`, and `tailwind-merge` as optional peers while the default root export still requires them.</comment>
<file context>
@@ -152,6 +172,17 @@
"react-dom": ">=19.0.0",
"tailwind-merge": ">=3.0.0"
},
+ "peerDependenciesMeta": {
+ "class-variance-authority": {
+ "optional": true
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.attw.json:
- Line 2: Update the excludeEntrypoints entry to use the exact export key format
by adding the missing "./" prefix: change the string in the excludeEntrypoints
array from "styles/tailwind.css" to "./styles/tailwind.css" so it matches
AreTheTypesWrong's expected keys (refer to the excludeEntrypoints property in
.attw.json).
In `@package.json`:
- Around line 175-185: The peerDependenciesMeta currently marks
class-variance-authority, clsx, and tailwind-merge as optional which breaks the
default entry that imports src/styles/styled.ts; change package.json so those
three remain required peerDependencies for the default export (i.e., remove them
from peerDependenciesMeta optional list) OR refactor the styled runtime into its
own export/package that imports src/styles/styled.ts and declare those three as
peers only on that new export; keep react-mentions-ts/core referencing
src/styles/core.ts as optional-peer-safe.
In `@README.md`:
- Line 412: Replace the smart quotes in the `@source` directive example in
README.md for the optional helper (react-mentions-ts/styles/tailwind.css) with
plain ASCII double quotes so the snippet reads `@source` "../dist";; locate the
example showing the `@source` “../dist” directive and update the quotation
characters to plain quotes to ensure valid CSS when copied.
In `@src/core.ts`:
- Around line 1-23: The core index is missing a re-export for the MentionProps
type; add MentionProps to the exported type list in src/core.ts so consumers can
import { MentionProps } from './core' instead of deep-importing ./core/Mention;
update the type export block (the list containing MentionsInputProps,
MentionComponentProps, etc.) to include MentionProps alongside those existing
type names to expose the public prop type.
In `@src/coreEntry.spec.tsx`:
- Around line 91-102: The test currently only examines collectCoreSourceGraph()
output (graphText) which misses regressions in the built /core artifacts and
only matches exact specifiers; update the test to either read and assert the
emitted core bundle(s) (e.g., the built dist/core.* files) for absence of
'tailwind-merge' and 'class-variance-authority' and imports to styled helpers,
or normalize import specifiers before matching by resolving/normalizing paths
(so checks like "from './styles/styled'" also match "../styles/styled" and
deeper variants); specifically modify the test around
collectCoreSourceGraph/graphText and the expect checks to (1) load the emitted
core artifact content and assert it does not contain the helper names/specifier
patterns, or (2) transform each specifier (e.g., path.normalize or a regex)
before using expect(...).not.toContain for 'tailwind-merge',
'class-variance-authority', "from './styles/styled'" and "from './utils/cn'".
In `@src/HighlighterBase.tsx`:
- Around line 180-222: The caret marker is only injected inside textIteratee, so
collapsed caret positions immediately before/after mentions are missed; update
mentionIteratee (used by iterateMentionsMarkup) to detect when
caretPositionInMarkup falls at the mention boundary (use plainTextIndex and the
mention's display length/indices) and push renderCaretMarker() into the correct
components array (or split the mention into prefix/suffix similar to
textIteratee) before/after calling getMentionComponentForMatch; ensure the same
componentKey handling and final splice into resultComponents is preserved so
caret placement is consistent with the existing logic around components,
resultComponents, renderSubstring, and renderCaretMarker.
In `@src/MentionsInput.performance.spec.tsx`:
- Line 99: Add a short inline comment next to the performance ceiling assertions
(e.g., the expect(metrics.getPlainTextCalls).toBeLessThanOrEqual(2) at the
current location and the similar assertions at the other two spots) documenting
why the allowed ceiling was raised: describe the fixture/behavior change that
causes the extra getPlainText calls, what the new expected upper bound
represents, and that these constants act as regression guards for the fixtures
so future readers understand the rationale for the higher threshold.
In `@src/MentionsInputBase.tsx`:
- Around line 1858-1874: The debounce callback currently calls
provideData(queryInfo.query) directly so any synchronous throw escapes and
leaves the query loading; change executeQuery to evaluate provideData inside a
try/catch and convert its result into a promise before calling
updateSuggestions: e.g. wrap provideData(queryInfo.query) with
Promise.resolve(...) in the try branch and on catch create a rejected promise
(Promise.reject(err)) so that updateSuggestions(queryId, childIndex, queryInfo,
resultPromise, controller) receives a promise in all cases and can route errors
through the existing query error path; use the existing symbols executeQuery,
provideData, updateSuggestions, getDataProvider, controller, queryId,
childIndex, and queryInfo to locate and modify the code.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d4fa2867-da24-4ab1-bb5b-24710f6b698d
📒 Files selected for processing (35)
.attw.jsonREADME.mdpackage.jsonsrc/Highlighter.tsxsrc/HighlighterBase.tsxsrc/LoadingIndicator.tsxsrc/LoadingIndicatorBase.tsxsrc/Mention.tsxsrc/MentionBase.tsxsrc/MentionsInput.performance.spec.tsxsrc/MentionsInput.spec.tsxsrc/MentionsInput.tsxsrc/MentionsInputBase.tsxsrc/MentionsInputDerived.tssrc/MentionsInputEditing.tssrc/MentionsInputQueryState.tssrc/MentionsInputSelectors.tssrc/MentionsInputView.tsxsrc/Suggestion.tsxsrc/SuggestionBase.tsxsrc/SuggestionsOverlay.tsxsrc/SuggestionsOverlayBase.tsxsrc/core.tssrc/core/Mention.tsxsrc/core/MentionsInput.tsxsrc/coreEntry.spec.tsxsrc/styles/core.tssrc/styles/joinClassNames.tssrc/styles/mergeStyles.tssrc/styles/styled.tssrc/styles/tailwindMergeClassNames.tssrc/styles/types.tssrc/utils/cn.tssrc/utils/index.tstsdown.config.ts
💤 Files with no reviewable changes (2)
- src/utils/cn.ts
- src/utils/index.ts
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "excludeEntrypoints": ["styles/tailwind.css"] | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In AreTheTypesWrong .attw.json, should excludeEntrypointsvalues for subpath exports use the exact export key (e.g.,"./styles/tailwind.css") or omit "./"(e.g.,"styles/tailwind.css")?
💡 Result:
In AreTheTypesWrong's .attw.json, excludeEntrypoints values for subpath exports should use the exact export key including the "./" prefix, such as "./styles/tailwind.css". The official CLI documentation and example configurations consistently show entrypoint paths with the ./ prefix. For example: - CLI usage: attw --pack . --exclude-entrypoints styles.css comments explain this excludes "./styles.css". - .attw.json example: "excludeEntrypoints": ["./internal"]. - --entrypoints . one two three maps to [".", "./one", "./two", "./three"]. Subpath exports in package.json are defined with ./ (e.g., "./styles/tailwind.css": "./dist/styles/tailwind.css"), and attw discovers/matches them as full subpaths from the package root, requiring the ./ prefix in excludeEntrypoints for exact matching.
Citations:
- 1: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/packages/cli/README.md
- 2: https://context7.com/arethetypeswrong/arethetypeswrong.github.io/llms.txt
- 3: https://www.npmjs.com/package/@arethetypeswrong/cli
- 4: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/161725ad2e8957f109e44fb26b13c9d70f415c2f/packages/cli/README.md
- 5: https://linkedin.com/in/apartington
Use "./styles/tailwind.css" in excludeEntrypoints.
The current value "styles/tailwind.css" is missing the ./ prefix. AreTheTypesWrong requires excludeEntrypoints values to match the exact export key format, which includes the ./ prefix. Update to "./styles/tailwind.css".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.attw.json at line 2, Update the excludeEntrypoints entry to use the exact
export key format by adding the missing "./" prefix: change the string in the
excludeEntrypoints array from "styles/tailwind.css" to "./styles/tailwind.css"
so it matches AreTheTypesWrong's expected keys (refer to the excludeEntrypoints
property in .attw.json).
| "peerDependenciesMeta": { | ||
| "class-variance-authority": { | ||
| "optional": true | ||
| }, | ||
| "clsx": { | ||
| "optional": true | ||
| }, | ||
| "tailwind-merge": { | ||
| "optional": true | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n package.json | head -200Repository: hbmartin/react-mentions-ts
Length of output: 7227
🏁 Script executed:
# Check the exports structure and dependencies
cat package.json | jq '.exports, .dependencies, .peerDependencies, .peerDependenciesMeta' 2>/dev/null || cat package.json | grep -A 50 '"exports"' | head -60Repository: hbmartin/react-mentions-ts
Length of output: 909
🏁 Script executed:
# Find the main entry point and check its imports
git ls-files | grep -E '(src/|lib/)' | head -20Repository: hbmartin/react-mentions-ts
Length of output: 807
🏁 Script executed:
# Look for the styled.ts file mentioned in the comment
fd -t f 'styled\.ts' 2>/dev/null || find . -name 'styled.ts' -type f 2>/dev/nullRepository: hbmartin/react-mentions-ts
Length of output: 90
🏁 Script executed:
cat -n src/styles/styled.tsRepository: hbmartin/react-mentions-ts
Length of output: 4273
🏁 Script executed:
# Check the main index entry point
fd -t f 'index\.ts' -path 'src/*' | head -5Repository: hbmartin/react-mentions-ts
Length of output: 238
🏁 Script executed:
# Check if there's a core.ts file
fd -t f 'core\.ts' -path 'src/*' | head -5Repository: hbmartin/react-mentions-ts
Length of output: 238
🏁 Script executed:
# Check imports in the main index file
cat src/index.ts 2>/dev/null | head -50Repository: hbmartin/react-mentions-ts
Length of output: 821
🏁 Script executed:
# Search for imports of styled.ts
rg -n "from.*styled\'" --type ts --type tsxRepository: hbmartin/react-mentions-ts
Length of output: 162
🏁 Script executed:
# Check MentionsInput imports
rg -n "import.*styled" src/MentionsInput.tsx -A 2 -B 2Repository: hbmartin/react-mentions-ts
Length of output: 209
🏁 Script executed:
# Look for core.ts file
fd -type f 'core.ts'Repository: hbmartin/react-mentions-ts
Length of output: 240
🏁 Script executed:
# Check what gets exported from the main components
rg -n "styledStyles\|styled\|tailwindMerge\|cva" src/MentionsInput.tsx | head -20Repository: hbmartin/react-mentions-ts
Length of output: 52
🏁 Script executed:
# Check what tailwindMergeClassNames imports
cat -n src/styles/tailwindMergeClassNames.tsRepository: hbmartin/react-mentions-ts
Length of output: 432
🏁 Script executed:
# Search for imports of styledStyles
rg -n "styledStyles" src/Repository: hbmartin/react-mentions-ts
Length of output: 1195
🏁 Script executed:
# Check if there's a core entry point file
fd 'core\.ts$' src/Repository: hbmartin/react-mentions-ts
Length of output: 100
🏁 Script executed:
# Look for what the build system exports for core and main
cat tsdown.config.ts 2>/dev/null || find . -name "tsdown.config*" -type fRepository: hbmartin/react-mentions-ts
Length of output: 504
🏁 Script executed:
# Check what's in the core entry point
cat -n src/core.tsRepository: hbmartin/react-mentions-ts
Length of output: 992
🏁 Script executed:
# Check what's in styles/core.ts
cat -n src/styles/core.tsRepository: hbmartin/react-mentions-ts
Length of output: 5730
🏁 Script executed:
# Check if core/MentionsInput imports styled or core styles
cat -n src/core/MentionsInput.tsx | head -50Repository: hbmartin/react-mentions-ts
Length of output: 402
🏁 Script executed:
# List what's in the core directory
find src/core -type f -name "*.tsx" | head -20Repository: hbmartin/react-mentions-ts
Length of output: 117
🏁 Script executed:
# Check if any core components import the styled.ts (which uses optional deps)
rg -l "from.*styles/styled" src/core/Repository: hbmartin/react-mentions-ts
Length of output: 52
🏁 Script executed:
# Verify if default MentionsInput uses styled
head -20 src/MentionsInput.tsxRepository: hbmartin/react-mentions-ts
Length of output: 350
Don't make the styled runtime helpers optional peers.
The default entry imports class-variance-authority, clsx, and tailwind-merge directly through src/styles/styled.ts, making them hard runtime dependencies. The react-mentions-ts/core entry uses src/styles/core.ts instead, which has no dependency on these packages and can safely treat them as optional. However, marking all three as optional peer dependencies removes the install-time signal for the default entry, creating a runtime risk for consumers whose package managers do not auto-install optional peers.
Either keep these as required peer dependencies for the default entry, or split the styled entry behind a separate package/export that explicitly declares the dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 175 - 185, The peerDependenciesMeta currently
marks class-variance-authority, clsx, and tailwind-merge as optional which
breaks the default entry that imports src/styles/styled.ts; change package.json
so those three remain required peerDependencies for the default export (i.e.,
remove them from peerDependenciesMeta optional list) OR refactor the styled
runtime into its own export/package that imports src/styles/styled.ts and
declare those three as peers only on that new export; keep
react-mentions-ts/core referencing src/styles/core.ts as optional-peer-safe.
| ``` | ||
|
|
||
| The optional helper `react-mentions-ts/styles/tailwind.css` only declares an `@source “../dist”;` directive so Tailwind v4 can detect the library's utility classes inside `node_modules/react-mentions-ts/dist`. Including it keeps your Tailwind config clean and avoids adding explicit `content` globs for the package. | ||
| The optional helper `react-mentions-ts/styles/tailwind.css` is for the default styled entry. It only declares an `@source “../dist”;` directive so Tailwind v4 can detect the library's utility classes inside `node_modules/react-mentions-ts/dist`. Including it keeps your Tailwind config clean and avoids adding explicit `content` globs for the package. |
There was a problem hiding this comment.
Use plain quotes in the @source example.
The inline directive currently shows smart quotes around ../dist, so a copy/paste into CSS will be invalid.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 412, Replace the smart quotes in the `@source` directive
example in README.md for the optional helper
(react-mentions-ts/styles/tailwind.css) with plain ASCII double quotes so the
snippet reads `@source` "../dist";; locate the example showing the `@source`
“../dist” directive and update the quotation characters to plain quotes to
ensure valid CSS when copied.
| export { default as MentionsInput } from './core/MentionsInput' | ||
| export { default as Mention } from './core/Mention' | ||
| export type { | ||
| MentionsInputProps, | ||
| MentionsInputClassNames, | ||
| MentionComponentProps, | ||
| MentionDataItem, | ||
| MentionRenderEmpty, | ||
| MentionRenderError, | ||
| MentionSearchContext, | ||
| MentionsInputChangeEvent, | ||
| MentionsInputChangeHandler, | ||
| MentionsInputChangeTrigger, | ||
| MentionsInputChangeTriggerType, | ||
| MentionSerializer, | ||
| MentionSerializerMatch, | ||
| MentionSelection, | ||
| MentionSelectionState, | ||
| SuggestionQueryState, | ||
| } from './types' | ||
| export { default as createMarkupSerializer } from './utils/createMarkupSerializer' | ||
| export { makeTriggerRegex } from './utils/makeTriggerRegex' | ||
| export { default as getSubstringIndex } from './utils/getSubstringIndex' |
There was a problem hiding this comment.
Re-export MentionProps from the public /core entry.
src/core/Mention.tsx already exposes the prop type, but /core drops it. That forces TypeScript consumers to deep-import an internal file just to type a <Mention /> wrapper.
Proposed fix
export { default as MentionsInput } from './core/MentionsInput'
export { default as Mention } from './core/Mention'
+export type { MentionProps } from './core/Mention'
export type {
MentionsInputProps,
MentionsInputClassNames,
MentionComponentProps,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core.ts` around lines 1 - 23, The core index is missing a re-export for
the MentionProps type; add MentionProps to the exported type list in src/core.ts
so consumers can import { MentionProps } from './core' instead of deep-importing
./core/Mention; update the type export block (the list containing
MentionsInputProps, MentionComponentProps, etc.) to include MentionProps
alongside those existing type names to expose the public prop type.
| it('keeps styled-only helpers out of the core source graph', () => { | ||
| const graph = collectCoreSourceGraph() | ||
| const graphText = [...graph] | ||
| // eslint-disable-next-line security/detect-non-literal-fs-filename -- source graph traversal only reads discovered repo source files. | ||
| .map((file) => `${file}\n${readFileSync(file, 'utf8')}`) | ||
| .join('\n') | ||
|
|
||
| expect(graphText).not.toContain('tailwind-merge') | ||
| expect(graphText).not.toContain('class-variance-authority') | ||
| expect(graphText).not.toContain("from './styles/styled'") | ||
| expect(graphText).not.toContain("from './utils/cn'") | ||
| }) |
There was a problem hiding this comment.
This only guards the source graph, not the shipped /core entry.
A tsdown/exports regression can still leak styled helpers into dist/core.* while this passes, and the path checks only match ./styles/styled / ./utils/cn even though nested core files would import them as ../.... Please assert on the emitted core artifact, or at least normalize specifiers before matching.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/coreEntry.spec.tsx` around lines 91 - 102, The test currently only
examines collectCoreSourceGraph() output (graphText) which misses regressions in
the built /core artifacts and only matches exact specifiers; update the test to
either read and assert the emitted core bundle(s) (e.g., the built dist/core.*
files) for absence of 'tailwind-merge' and 'class-variance-authority' and
imports to styled helpers, or normalize import specifiers before matching by
resolving/normalizing paths (so checks like "from './styles/styled'" also match
"../styles/styled" and deeper variants); specifically modify the test around
collectCoreSourceGraph/graphText and the expect checks to (1) load the emitted
core artifact content and assert it does not contain the helper names/specifier
patterns, or (2) transform each specifier (e.g., path.normalize or a regex)
before using expect(...).not.toContain for 'tailwind-merge',
'class-variance-authority', "from './styles/styled'" and "from './utils/cn'".
| const textIteratee = (substr: string, index: number, _substrPlainTextIndex: number) => { | ||
| if ( | ||
| isNumber(caretPositionInMarkup) && | ||
| caretPositionInMarkup >= index && | ||
| caretPositionInMarkup <= index + substr.length | ||
| ) { | ||
| const splitIndex = caretPositionInMarkup - index | ||
| // Before the reassignment, components still points at resultComponents, so the push stores | ||
| // the substring that comes before the caret in the final output. | ||
| components.push(renderSubstring(substr.slice(0, splitIndex), substringComponentKey)) | ||
| substringComponentKey += 1 | ||
| // Reassigning component just switches the working array to a fresh list | ||
| // for the text after the caret; later we splice that array back in | ||
| // Without the initial push, the prefix would not land in resultComponents. | ||
| components = [renderSubstring(substr.slice(splitIndex), substringComponentKey)] | ||
| } else { | ||
| components.push(renderSubstring(substr, substringComponentKey)) | ||
| } | ||
| substringComponentKey += 1 | ||
| } | ||
|
|
||
| const mentionIteratee = ( | ||
| _markup: string, | ||
| _index: number, | ||
| plainTextIndex: number, | ||
| id: string, | ||
| display: string, | ||
| mentionChildIndex: number | ||
| ) => { | ||
| const key = generateComponentKey(componentKeys, id) | ||
| components.push( | ||
| getMentionComponentForMatch(id, display, mentionChildIndex, key, plainTextIndex) | ||
| ) | ||
| } | ||
|
|
||
| iterateMentionsMarkup(value, config, mentionIteratee, textIteratee) | ||
|
|
||
| // append a span containing a space, to ensure the last text line has the correct height | ||
| components.push(' ') | ||
|
|
||
| if (components !== resultComponents) { | ||
| resultComponents.push(renderCaretMarker(), ...components) | ||
| } |
There was a problem hiding this comment.
Render the caret marker at mention boundaries too.
The caret is only inserted from textIteratee(). A collapsed selection immediately before or after a mention never creates data-mentions-caret, so caret measurement and suggestion positioning break at those boundaries.
Proposed fix
+ let caretRendered = false
+
const textIteratee = (substr: string, index: number, _substrPlainTextIndex: number) => {
if (
isNumber(caretPositionInMarkup) &&
caretPositionInMarkup >= index &&
caretPositionInMarkup <= index + substr.length
) {
const splitIndex = caretPositionInMarkup - index
components.push(renderSubstring(substr.slice(0, splitIndex), substringComponentKey))
substringComponentKey += 1
+ caretRendered = true
components = [renderSubstring(substr.slice(splitIndex), substringComponentKey)]
} else {
components.push(renderSubstring(substr, substringComponentKey))
}
substringComponentKey += 1
}
const mentionIteratee = (
- _markup: string,
- _index: number,
+ markup: string,
+ index: number,
plainTextIndex: number,
id: string,
display: string,
mentionChildIndex: number
) => {
+ if (caretPositionInMarkup === index) {
+ components.push(renderCaretMarker())
+ caretRendered = true
+ }
const key = generateComponentKey(componentKeys, id)
components.push(
getMentionComponentForMatch(id, display, mentionChildIndex, key, plainTextIndex)
)
+ if (caretPositionInMarkup === index + markup.length) {
+ components.push(renderCaretMarker())
+ caretRendered = true
+ }
}
iterateMentionsMarkup(value, config, mentionIteratee, textIteratee)
+
+ if (!caretRendered && caretPositionInMarkup === value.length) {
+ components.push(renderCaretMarker())
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/HighlighterBase.tsx` around lines 180 - 222, The caret marker is only
injected inside textIteratee, so collapsed caret positions immediately
before/after mentions are missed; update mentionIteratee (used by
iterateMentionsMarkup) to detect when caretPositionInMarkup falls at the mention
boundary (use plainTextIndex and the mention's display length/indices) and push
renderCaretMarker() into the correct components array (or split the mention into
prefix/suffix similar to textIteratee) before/after calling
getMentionComponentForMatch; ensure the same componentKey handling and final
splice into resultComponents is preserved so caret placement is consistent with
the existing logic around components, resultComponents, renderSubstring, and
renderCaretMarker.
| emitPerformanceMetric('controlled-keystroke', metrics) | ||
|
|
||
| expect(metrics.getPlainTextCalls).toBe(0) | ||
| expect(metrics.getPlainTextCalls).toBeLessThanOrEqual(2) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Document why these performance ceilings increased.
These constants are now the regression guard for the fixtures. A short inline note about the extra expected calls would make future perf regressions much easier to interpret.
Also applies to: 425-425, 511-511
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/MentionsInput.performance.spec.tsx` at line 99, Add a short inline
comment next to the performance ceiling assertions (e.g., the
expect(metrics.getPlainTextCalls).toBeLessThanOrEqual(2) at the current location
and the similar assertions at the other two spots) documenting why the allowed
ceiling was raised: describe the fixture/behavior change that causes the extra
getPlainText calls, what the new expected upper bound represents, and that these
constants act as regression guards for the fixtures so future readers understand
the rationale for the higher threshold.
| const executeQuery = () => { | ||
| this._queryDebounceTimers.delete(childIndex) | ||
|
|
||
| const provideData = getDataProvider<Extra>(mentionChild.props.data, { | ||
| ignoreAccents, | ||
| maxSuggestions, | ||
| signal: controller.signal, | ||
| getSubstringIndex, | ||
| }) | ||
|
|
||
| void this.updateSuggestions( | ||
| queryId, | ||
| childIndex, | ||
| queryInfo, | ||
| provideData(queryInfo.query), | ||
| controller | ||
| ) |
There was a problem hiding this comment.
Route synchronous provider failures through the existing query error path.
provideData(queryInfo.query) is evaluated before updateSuggestions() starts. If a consumer's data provider throws synchronously, the exception escapes the debounce callback and the query stays stuck in the loading state.
Proposed fix
const executeQuery = () => {
this._queryDebounceTimers.delete(childIndex)
- const provideData = getDataProvider<Extra>(mentionChild.props.data, {
- ignoreAccents,
- maxSuggestions,
- signal: controller.signal,
- getSubstringIndex,
- })
-
- void this.updateSuggestions(
- queryId,
- childIndex,
- queryInfo,
- provideData(queryInfo.query),
- controller
- )
+ const results = Promise.resolve().then(() => {
+ const provideData = getDataProvider<Extra>(mentionChild.props.data, {
+ ignoreAccents,
+ maxSuggestions,
+ signal: controller.signal,
+ getSubstringIndex,
+ })
+
+ return provideData(queryInfo.query)
+ })
+
+ void this.updateSuggestions(queryId, childIndex, queryInfo, results, controller)
}📝 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.
| const executeQuery = () => { | |
| this._queryDebounceTimers.delete(childIndex) | |
| const provideData = getDataProvider<Extra>(mentionChild.props.data, { | |
| ignoreAccents, | |
| maxSuggestions, | |
| signal: controller.signal, | |
| getSubstringIndex, | |
| }) | |
| void this.updateSuggestions( | |
| queryId, | |
| childIndex, | |
| queryInfo, | |
| provideData(queryInfo.query), | |
| controller | |
| ) | |
| const executeQuery = () => { | |
| this._queryDebounceTimers.delete(childIndex) | |
| const results = Promise.resolve().then(() => { | |
| const provideData = getDataProvider<Extra>(mentionChild.props.data, { | |
| ignoreAccents, | |
| maxSuggestions, | |
| signal: controller.signal, | |
| getSubstringIndex, | |
| }) | |
| return provideData(queryInfo.query) | |
| }) | |
| void this.updateSuggestions(queryId, childIndex, queryInfo, results, controller) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/MentionsInputBase.tsx` around lines 1858 - 1874, The debounce callback
currently calls provideData(queryInfo.query) directly so any synchronous throw
escapes and leaves the query loading; change executeQuery to evaluate
provideData inside a try/catch and convert its result into a promise before
calling updateSuggestions: e.g. wrap provideData(queryInfo.query) with
Promise.resolve(...) in the try branch and on catch create a rejected promise
(Promise.reject(err)) so that updateSuggestions(queryId, childIndex, queryInfo,
resultPromise, controller) receives a promise in all cases and can route errors
through the existing query error path; use the existing symbols executeQuery,
provideData, updateSuggestions, getDataProvider, controller, queryId,
childIndex, and queryInfo to locate and modify the code.
…yled/backward-compatible. The split is now explicit: core uses dependency-free structural styles and class concatenation, while styled root keeps cva, clsx, and tailwind-merge in src/styles/styled.ts (line 42).
Updated packaging/build exports in package.json (line 11) and tsdown.config.ts (line 4), added core entry tests in src/coreEntry.spec.tsx (line 1), and removed the stale src/utils/cn.ts helper.
Summary by CodeRabbit
New Features
/coreentry point providing an unstyled component API../styles/tailwind.cssfor explicit styling imports.Documentation
Chores
6.0.0-next.1.Note
Add a dependency-free
react-mentions-ts/coreentry point alongside the existing styled entryBasevariants of all major components (MentionsInputBase,HighlighterBase,SuggestionsOverlayBase,SuggestionBase,MentionBase,LoadingIndicatorBase) that accept an injected style config instead of hard-coded Tailwind classes../coresubpath export in package.json and a new src/core.ts entry built by tsdown.config.ts, re-exporting core components with plain CSS class-based styles from src/styles/core.ts.react-mentions-ts) continues to use Tailwind-basedstyledStyles;class-variance-authority,clsx, andtailwind-mergeare now optional peer dependencies.cnutility from src/utils/index.ts and replaces barrel imports throughout with direct module imports.cnfrom the utils barrel will break; theSuggestionandSuggestionsOverlaycomponents now compose class names differently via the style config, which may change rendered class names.Macroscope summarized bbcdaaf.