-
-
Notifications
You must be signed in to change notification settings - Fork 29
fix: improve type support for isolated dependencies in pnpm #289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c67f4e4
d20f483
445e5cd
4b25c8a
731d6f9
a4e40a8
e2926e5
53144d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
{ | ||
"extends": "./tsconfig.json", | ||
"files": ["dist/esm/index.js"] | ||
"files": ["dist/esm/index.js"], | ||
"compilerOptions": { | ||
"allowImportingTsExtensions": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "module", | ||
"dependencies": { | ||
"@eslint/config-helpers": "file:../..", | ||
"typescript": "^5.9.3" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import { defineConfig, globalIgnores } from "@eslint/config-helpers"; | ||
|
||
export const ignoresConfig = globalIgnores([]); | ||
|
||
export default defineConfig([ignoresConfig]); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"compilerOptions": { | ||
"checkJs": true, | ||
"declaration": true, | ||
"module": "nodenext", | ||
"noEmit": true | ||
}, | ||
"files": ["test-eslint.config.js"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,8 @@ if (!newFilename) { | |
|
||
const oldSourceText = await readFile(filename, "utf-8"); | ||
const newSourceText = oldSourceText.replaceAll( | ||
'import("./types.ts")', | ||
'import("./types.cts")', | ||
' from "./types.ts";\n', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just double-checking -- are we sure there are no files that use |
||
' from "./types.cts";\n', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just double-checking: Are we sure there aren't any files that are still using |
||
); | ||
|
||
await writeFile(newFilename, newSourceText); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,19 +25,31 @@ const files = process.argv.slice(2); | |
files.forEach(filePath => { | ||
const lines = fs.readFileSync(filePath, "utf8").split(/\r?\n/gu); | ||
const typedefs = new Set(); | ||
|
||
const remainingLines = lines.filter(line => { | ||
if (!line.startsWith("/** @typedef {import")) { | ||
return true; | ||
} | ||
|
||
if (typedefs.has(line)) { | ||
return false; | ||
const importSources = new Set(); | ||
const outputLines = []; | ||
|
||
for (const line of lines) { | ||
const match = line.match( | ||
/^(?<start>\/\*\*\s*@typedef\s+\{)import\("(?<importSource>.+?)"\)(?<end>.*)/u, | ||
); | ||
if (!match) { | ||
// not a typedef, so just copy the line | ||
outputLines.push(line); | ||
} else if (!typedefs.has(line)) { | ||
// we haven't seen this typedef before, so process it | ||
typedefs.add(line); | ||
const { start, importSource, end } = match.groups; | ||
const importName = `_${importSource.replace(/\W/gu, "")}`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of using |
||
if (!importSources.has(importSource)) { | ||
// we haven't seen this import before, so add an @import comment | ||
importSources.add(importSource); | ||
outputLines.push( | ||
`/** @import * as ${importName} from "${importSource}"; */`, | ||
); | ||
} | ||
outputLines.push(`${start}${importName}${end}`); | ||
} | ||
} | ||
|
||
typedefs.add(line); | ||
return true; | ||
}); | ||
|
||
fs.writeFileSync(filePath, remainingLines.join("\n"), "utf8"); | ||
fs.writeFileSync(filePath, outputLines.join("\n"), "utf8"); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowImportingTsExtensions
is required for TypeScript to accept import statements from"./types.ts"
in the generateddist/esm/index.js
file:/** @import * as _typests from "./types.ts"; */
Another possibility to avoid
allowImportingTsExtensions
is importing from"./types.js"
instead. This will work if a dummy filedist/esm/types.js
is added to the package, similarly to what is done ineslint/markdown
:https://app.unpkg.com/@eslint/markdown@7.4.0/files/dist/esm/types.js