Skip to content

feat(correct-ts-specifiers): use JSSG#415

Open
AugustinMauroy wants to merge 3 commits intomainfrom
test
Open

feat(correct-ts-specifiers): use JSSG#415
AugustinMauroy wants to merge 3 commits intomainfrom
test

Conversation

@AugustinMauroy
Copy link
Copy Markdown
Member

@AugustinMauroy AugustinMauroy commented Mar 29, 2026

TODO:

  • clean little bit
  • update lock-file

@AugustinMauroy AugustinMauroy changed the title test feat(correct-ts-specifiers): use JSSG Mar 31, 2026
@AugustinMauroy AugustinMauroy marked this pull request as ready for review March 31, 2026 07:00
Copy link
Copy Markdown
Member

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this! But the PR is very difficult to review because of all the formatting changes. Could they be extracted into their own PR first, we just land that right away since there's no real changes, and then merge main into this one to collapse them away?

Comment on lines +9 to +10
type ResolveSpecifier =
typeof import('./resolve-specifier.ts').resolveSpecifier;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want

Suggested change
type ResolveSpecifier =
typeof import('./resolve-specifier.ts').resolveSpecifier;
import type { ResolveSpecifier } from './resolve-specifier.ts');

Node ignores type imports, so that doesn't cause the module to be adversely loaded into the ModuleCache.

Comment on lines 23 to 31
before(() => {
const access = mock.fn<FSAccess>();
({ mock: mock__access } = access);
mock.module('node:fs/promises', {
mock.module('fs', {
namedExports: {
access,
constants,
promises: { access },
},
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up: I subsequently realised before is actually superfluous (not sure if it actually has any use-case 🤔).

Suggested change
before(() => {
const access = mock.fn<FSAccess>();
({ mock: mock__access } = access);
mock.module('node:fs/promises', {
mock.module('fs', {
namedExports: {
access,
constants,
promises: { access },
},
});
const access = mock.fn<FSAccess>();
({ mock: mock__access } = access);
mock.module('node:fs/promises', {
mock.module('fs', {
namedExports: {
access,
constants,
promises: { access },
},
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants