feat: support non identifier classnames #309

apple-yagi · 2026-01-21T14:51:57Z

Background

At a social gathering after a certain study session held in Fukuoka the other day, the topic of css-modules-kit came up. At that time, I heard that css-modules-kit does not allow kebab-case, so I couldn't use it. I investigated whether it was not allowed, and found that it was, so I submitted a PR.

Description

I have modified it to generate type definitions using string literals, referring to #177. This allows defining classnames that are not JS identifiers.
Since namedExports can only use JS identifiers, this will only support defaultExports.

As an implementation issue, the logic has become complex because we needed to manage mappings doubly (one for definition jumps and one for renaming) (this change caused various bugs).
While I believe this feature addition is good for the further adoption of css-modules-kit, I also want to avoid making the code so complex that it becomes unmaintainable in the long run. I would like to respect mizdra's opinion on this 🙏

Related issues

#176

AI Assistance

This PR was written with assistance from Codex (ChatGPT).

changeset-bot · 2026-01-21T14:52:01Z

⚠️ No Changeset found

Latest commit: 51719ff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

mizdra · 2026-01-22T01:56:39Z

Great work! This is a big change. I'll review it carefully. It may take some time.

mizdra · 2026-01-22T09:53:24Z

FYI: CSS Modules Kit has flaky tests (#301). Even if your implementation is correct, CI may fail.

apple-yagi · 2026-01-22T09:58:11Z

Looking back at the code, I noticed there was unnecessary code and deleted it 🙏

mizdra · 2026-01-24T14:54:27Z

@apple-yagi I've made some changes to the main branch—could you please merge or rebase the main branch?

Fix flaky test (packages/ts-plugin/e2e-test/file-operation.test.ts > updating file) #314
- Some flaky tests have been fixed
Fix incorrect Definition Preview #315
- feat: support non identifier classnames #309 appears to have contained bug fixes performed by Codex.
- It's not ideal to combine bug fixes and feature additions in a single Pull Request, so I addressed the bug in Fix incorrect Definition Preview #315 instead.

packages/ts-plugin/e2e-test/feature/rename-symbol.test.ts

mizdra · 2026-01-24T17:14:53Z

While I believe this feature addition is good for the further adoption of css-modules-kit, I also want to avoid making the code so complex that it becomes unmaintainable in the long run. I would like to respect mizdra's opinion on this 🙏

It certainly looks like a bit of complex code. I'll think deeply about whether there's a way to replace it with simpler code.

apple-yagi · 2026-01-25T13:11:44Z

@mizdra I believe all functions are now working correctly after fixing the bug in rename.
I don't think I can improve the quality of this code further, so I apologize, but I'll leave the rest to you.
Good luck! 🙇

…#309 This is the issue fixed in mizdra#317.

mizdra · 2026-01-25T16:42:17Z

packages/ts-plugin/src/language-service/feature/definition-and-bound-span.ts

+  return result;
+}
+
 export function getDefinitionAndBoundSpan(


Honestly, I still don't understand what this function is trying to do. Is it doing something that contributes to supporting non-identifier classnames?

When type definitions are made into string literals, tsserver cannot return definitions (it might be a bug in some tool, but I don't know). Therefore, I am manually mapping the definitions(I had Codex write the code).

Hmm, shouldn't we investigate that bug first? If it's a dependency bug and fixable, fixing it is the best approach.

If the dependency bug is difficult to fix or would take a long time to resolve, I think implementing a workaround to bypass it is worthwhile. Otherwise, I'd prefer to avoid implementing a complex workaround.

I'll investigate this bug. At this stage, I suspect it's a bug in tsserver or Volar.js.

Yes, I agree.
While organizing the discussion, I thought of another implementation method, so I'll create a new Pull Request and commit step by step. When defining types with string literals, I feel that single quotes are also recognized as part of the type. That might be a specification of tsserver (I briefly read the tsserver implementation but couldn't quite understand it).
With that in mind, I'll try implementing it using tsserver.

mizdra · 2026-01-25T16:59:34Z

packages/core/src/dts-generator.ts

I refactored dts-generator.ts to avoid breaking the code (1340313...12f5b4b). Additionally, I fixed the regression (c4dbe5b). As a result, it seems adding new mappings is no longer necessary.

By the way, you explained the following in #309 (comment):

As an implementation issue, the logic has become complex because we needed to manage mappings doubly

What does "managing mappings doubly" mean? Does it mean one more element is added to the mapping array? Looking at the changes in dts-generator.ts, it doesn't appear that any additional elements are being added to the mapping array.

@mizdra Sorry, Initially, when I submitted the PR, I thought that both quoted and unquoted mappings were necessary for string literals like styles['a-1'], because tsserver returns spans including quotes

css-modules-kit/packages/core/src/dts-generator.ts

Lines 262 to 275 in 96d2d6f

text += ` '`;

mapping.sourceOffsets.push(token.loc.start.offset);

const quoteStart = base + 2;

const keyStart = base + 3;

mapping.generatedOffsets.push(quoteStart);

mapping.lengths.push(token.name.length);

// Include quotes and `:` so string-literal ranges map correctly.

mapping.generatedLengths!.push(token.name.length + 3);

// Map the inner content so rename ranges without quotes still resolve.

mapping.sourceOffsets.push(token.loc.start.offset);

mapping.generatedOffsets.push(keyStart);

mapping.lengths.push(token.name.length);

mapping.generatedLengths!.push(token.name.length);

text += `${token.name}': '' as readonly string,\n`;

.
However, I realized that go-to-definition works even without double mapping because getDefinitionAndBoundSpan normalizes def.name and replaces it with the CSS location (https://github.com/apple-yagi/css-modules-kit/blob/c4dbe5b554281a3501ec0cb6544cbc62875e6ba6/packages/ts-plugin/src/language-service/feature/definition-and-bound-span.ts#L110-L132). Therefore, I removed it.

Incidentally, performing double mapping allows tsserver's getDefinitionAndBoundSpan to work partially correctly. For example, jumps for styles.a_1; or styles['a-1']; work correctly (this was my original idea). However, bugs occur with duplicate class names like styles.a_2, @import, @value ... from '...';, and symbol renaming, making the implementation increasingly complex.

apple-yagi force-pushed the feat/support-non-identifier-classnames branch from 96d2d6f to 3e6cbfb Compare January 21, 2026 14:57

mizdra added the Type: Add Add new features. label Jan 22, 2026

mizdra reviewed Jan 24, 2026

View reviewed changes

packages/ts-plugin/e2e-test/feature/rename-symbol.test.ts Show resolved Hide resolved

apple-yagi force-pushed the feat/support-non-identifier-classnames branch 3 times, most recently from 81f5cfb to 76ca2ba Compare January 25, 2026 12:56

feat: support non identifier classnames

1340313

apple-yagi force-pushed the feat/support-non-identifier-classnames branch from 76ca2ba to 1340313 Compare January 25, 2026 12:59

mizdra added 5 commits January 26, 2026 01:06

test: refactor test

8e5806f

refactor: refactoring to make changes easier to understand

cf34884

refactor: refactor test

1d4f1f3

refactor: generatedLengths can be omitted when it is equal to length

12f5b4b

fix: fix regression that occurred during the implementation of mizdra…

c4dbe5b

…#309 This is the issue fixed in mizdra#317.

mizdra reviewed Jan 25, 2026

View reviewed changes

refactor: remove dedupeDefinitions

51719ff

apple-yagi mentioned this pull request Jan 27, 2026

feat: support non identifier classnames 2 #320

Draft

	text += ` '`;
	mapping.sourceOffsets.push(token.loc.start.offset);
	const quoteStart = base + 2;
	const keyStart = base + 3;
	mapping.generatedOffsets.push(quoteStart);
	mapping.lengths.push(token.name.length);
	// Include quotes and `:` so string-literal ranges map correctly.
	mapping.generatedLengths!.push(token.name.length + 3);
	// Map the inner content so rename ranges without quotes still resolve.
	mapping.sourceOffsets.push(token.loc.start.offset);
	mapping.generatedOffsets.push(keyStart);
	mapping.lengths.push(token.name.length);
	mapping.generatedLengths!.push(token.name.length);
	text += `${token.name}': '' as readonly string,\n`;

Uh oh!

feat: support non identifier classnames #309

Are you sure you want to change the base?

feat: support non identifier classnames #309

Uh oh!

Conversation

apple-yagi commented Jan 21, 2026

Background

Description

Related issues

AI Assistance

Uh oh!

changeset-bot bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

mizdra commented Jan 22, 2026

Uh oh!

mizdra commented Jan 22, 2026

Uh oh!

apple-yagi commented Jan 22, 2026

Uh oh!

mizdra commented Jan 24, 2026

Uh oh!

Uh oh!

mizdra commented Jan 24, 2026

Uh oh!

apple-yagi commented Jan 25, 2026

Uh oh!

mizdra Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

apple-yagi Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

mizdra Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

apple-yagi Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

mizdra Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

apple-yagi Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

apple-yagi Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Jan 21, 2026 •

edited

Loading