Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add preserveConfigExtension option #232

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+23 −7
Open

feat: add preserveConfigExtension option #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/reference/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ interface Options {
* @default 'info'
*/
logLevel?: LogLevel
/**
* If true, preserves the extension of the `velite.config.[ts|js]` file in the output `index.d.ts` file.
* Useful if `"moduleResolution": "NodeNext"` where extension-less files aren't supported.
* @default false
*/
preserveConfigExtension?: boolean
Copy link

Choose a reason for hiding this comment

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

suggestion (documentation): Consider adding an explicit type annotation for consistency

Other options in this interface, like logLevel, have explicit type annotations. Adding : boolean here would maintain consistency.

Suggested change
preserveConfigExtension?: boolean
preserveConfigExtension?: boolean;

Copy link
Author

Choose a reason for hiding this comment

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

Adding the semicolon makes it inconsistent with the rest of the interface.

Copy link

Choose a reason for hiding this comment

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

Thank you for your feedback, we will generate fewer comments like this in the future according to the following instructions:

- Ensure the comment addresses a significant issue or improvement in the code or documentation.
- Avoid comments that suggest minor stylistic changes unless they significantly impact readability or maintainability.
- Check if the comment provides a clear rationale for the suggestion and if it aligns with existing code conventions or guidelines.

}
```

Expand Down
14 changes: 10 additions & 4 deletions src/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { mkdir, rm } from 'node:fs/promises'
import { join, normalize } from 'node:path'
import glob from 'fast-glob'
import micromatch from 'micromatch'
import { mkdir, rm } from 'node:fs/promises'
import { join, normalize } from 'node:path'
import { reporter } from 'vfile-reporter'

import { assets } from './assets'
Expand Down Expand Up @@ -241,14 +241,20 @@ export interface Options {
* @default false
*/
strict?: boolean
/**
* If true, preserves the extension of the `velite.config` file in the output `index.d.ts` file.
* Useful if `"moduleResolution": "NodeNext"` where extension-less files aren't supported.
* @default false
*/
preserveConfigExtension?: boolean
}

/**
* Build contents
* @param options build options
*/
export const build = async (options: Options = {}): Promise<Record<string, unknown>> => {
const { config: configFile, clean, logLevel, strict } = options
const { config: configFile, clean, logLevel, strict, preserveConfigExtension = false } = options
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Action Required: Add Tests for preserveConfigExtension Option

The preserveConfigExtension option is implemented in the build and output functions and is documented in the API reference. However, no tests have been found to verify its functionality.

  • Implementation Locations:
    • src/build.ts
    • src/output.ts
    • docs/reference/api.md

Recommendation:
Please add tests to ensure that the preserveConfigExtension option behaves as expected and maintains the desired functionality.

🔗 Analysis chain

LGTM: preserveConfigExtension option implemented in build function.

The changes correctly implement the new option in the build function and pass it to the outputEntry function. The default value of false ensures backward compatibility.

To ensure the preserveConfigExtension option is correctly implemented throughout the codebase, let's verify its usage:

Please review the results of these checks to ensure the option is fully implemented and tested.

Also applies to: 281-281

Action Required: Update and Re-run Verification Scripts

The previous verification scripts failed because ripgrep does not recognize typescript as a valid file type. Please use ts instead. Below are the updated scripts:

Please execute these updated scripts to continue the verification process.

Action Required: Further Verification Needed for preserveConfigExtension Implementation

The recent verification did not locate any instances of preserveConfigExtension within the src or test directories. To thoroughly confirm its implementation, please execute the following comprehensive search scripts:

Please run these scripts to gather additional information on the usage and implementation of the preserveConfigExtension option.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of preserveConfigExtension option

# Test 1: Check if outputEntry function signature has been updated
echo "Test 1: Checking outputEntry function signature"
rg --type typescript "function outputEntry.*preserveConfigExtension" src

# Test 2: Verify preserveConfigExtension is used in the output generation logic
echo "Test 2: Checking usage of preserveConfigExtension in output generation"
rg --type typescript "preserveConfigExtension.*\.js[\"']?" src

# Test 3: Ensure tests have been updated or added for the new option
echo "Test 3: Checking for updated or new tests"
rg --type typescript "test.*preserveConfigExtension" test

Length of output: 655

Script:

#!/bin/bash
# Description: Verify the implementation of preserveConfigExtension option with corrected file types

# Test 1: Check if outputEntry function signature has been updated
echo "Test 1: Checking outputEntry function signature"
rg --type ts "function outputEntry.*preserveConfigExtension" src

# Test 2: Verify preserveConfigExtension is used in the output generation logic
echo "Test 2: Checking usage of preserveConfigExtension in output generation"
rg --type ts "preserveConfigExtension.*\.js['\"]?" src

# Test 3: Ensure tests have been updated or added for the new option
echo "Test 3: Checking for updated or new tests"
rg --type ts "test.*preserveConfigExtension" test

Length of output: 526

Script:

#!/bin/bash
# Description: Comprehensive search for 'preserveConfigExtension' usage across the entire codebase

echo "Test 1: Searching for 'preserveConfigExtension' in all files"
rg "preserveConfigExtension" .

echo "Test 2: Searching for 'preserveConfigExtension' in non-TypeScript files"
rg "preserveConfigExtension" --type-not ts

echo "Test 3: Searching for any references or related configurations"
rg "preserveConfigExtension" src || rg "preserveConfigExtension" test

Length of output: 1901


logLevel != null && logger.set(logLevel)

Expand All @@ -272,7 +278,7 @@ export const build = async (options: Options = {}): Promise<Record<string, unkno
await mkdir(output.data, { recursive: true })
await mkdir(output.assets, { recursive: true })

await outputEntry(output.data, configPath, collections)
await outputEntry(output.data, configPath, collections, preserveConfigExtension)

logger.log('initialized', begin)

Expand Down
10 changes: 7 additions & 3 deletions src/output.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,18 @@ export const emit = async (path: string, content: string, log?: string): Promise
* @param dest output destination directory
* @param configPath resolved config file path
* @param collections collection options
* @param preserveConfigExtension if true, preserves the extension of the `velite.config` file in the output `index.d.ts` file.
*/
export const outputEntry = async (dest: string, configPath: string, collections: Collections): Promise<void> => {
export const outputEntry = async (dest: string, configPath: string, collections: Collections, preserveConfigExtension: boolean = false): Promise<void> => {
const begin = performance.now()

// generate entry according to `config.collections`
const configModPath = relative(dest, configPath)
let configModPath = relative(dest, configPath)
.replace(/\\/g, '/') // replace windows path separator
.replace(/\.[mc]?[jt]s$/i, '') // remove extension

if (!preserveConfigExtension) {
configModPath = configModPath.replace(/\.[mc]?[jt]s$/i, '') // remove extension
}

const entry: string[] = []
const dts: string[] = [`import config from '${configModPath}'\n`]
Expand Down