[bugfix] Ensured that normalizeFileExt ignores .css.d.ts files

[ijlee2]

Background

I created sample-v2-addon to investigate how we can introduce CSS modules to v2 addons (while supporting Glint). The key is, for each CSS file *.css, to create the corresponding declaration file *.css.d.ts.

sample-v2-addon
├── src
│   └── components
│       ├── navigation-menu.css
│       ├── navigation-menu.css.d.ts
│       ├── navigation-menu.hbs
│       └── navigation-menu.ts
...

When the addon's publicEntrypoints and appReexports include the "normal" wildcard pattern components/**/*.js, the resulting dist will include files that are not supposed to be present:

sample-v2-addon
├── dist
│   └── components
│       ├── navigation-menu.css.d.d.ts ❌
│       ├── navigation-menu.css.d.d.ts.map ❌
│       ├── navigation-menu.css.d.js ❌
│       ├── navigation-menu.css.d.js.map ❌
│       ├── navigation-menu.d.ts
│       ├── navigation-menu.js
│       └── navigation-menu.js.map
...

I worked around the problem by providing the pattern components/**/!(*.css.d).js, but find the additional complexity in regular expression to be a tech risk (not easy to understand—what is .css.d.js?—and maintain):

sample-v2-addon
├── dist
│   └── components
│       ├── navigation-menu.d.ts
│       ├── navigation-menu.d.ts.map
│       ├── navigation-menu.js
│       └── navigation-menu.js.map
...

Proposed solution

I suspect, the current implementation of normalizeFileExt didn't consider file extensions that include .ts as a substring.

function normalizeFileExt(fileName: string) {
  return fileName.replace(/\.ts|\.hbs|\.gts|\.gjs$/, '.js');
}

By making an early exit, we can consider *.ts cases and define a more specific rule that ignores .css.d.ts.

function normalizeFileExt(fileName: string) {
  if (fileName.endsWith('.ts')) {
    // Match .ts but not .d.ts
    const regex = /(^.?|\.[^d]|[^.]d|[^.][^d])\.ts$/;

    return fileName.replace(regex, '$1.js');
  }

  return fileName.replace(/\.hbs|\.gts|\.gjs$/, '.js');
}

Admittedly, the regular expression /(^.?|\.[^d]|[^.]d|[^.][^d])\.ts$/, copied from StackOverflow, is complex and forms another tech risk. Ideally, unit tests for normalizeFileExt would be present to document its input and output. (I didn't know in which package such tests can be written.)

[ef4]

Thanks, this makes sense. Typescript caused a lot of headaches when they picked .d.ts to mean a different thing than .ts.

[ef4]

Can you please check your use case and see if this implementation also solves it?

return fileName.replace(/(?<!\.d)\.ts|\.hbs|\.gts|\.gjs$/, '.js');

The idea here is "negative lookbehind assertions".

If this works, we can stick with only a single regex replace instead of nesting them.

[ijlee2]

Thank you, I checked that your solution works in ijlee2/embroider-css-modules#46.