[Feature] Add <SEPARATOR> special word#296
Conversation
|
Hi @ayusharma, @byara |
|
This will need my additional changes (joutvhu/prettier-plugin-sort-imports#2) before merging, as adding tests revealed some critical issues. |
|
I changes the base branch to v6. Let's release it there. |
|
@byara I added tests |
Add tests for the separator special word
# Conflicts: # src/utils/get-sorted-nodes.ts
There was a problem hiding this comment.
this doesnt have any comments? I already included similar tests in ImportsManualSeparated, so i think these Separator tests can be removed or merged into that folder.
byara
left a comment
byara
left a comment
There was a problem hiding this comment.
Thank you for the PR 👏 really appreciate the effort you have put into this!
I left a comment, could you please take a look?
There was a problem hiding this comment.
Pull Request Overview
This PR adds a <SEPARATOR> placeholder to the import-sorting plugin, allowing users to manually inject blank lines between import groups.
- Introduce a new
SEPARATOR_SPECIAL_WORDconstant and expose it insrc/constants.ts - Extend
get-sorted-nodes-by-import-order.tsto recognize<SEPARATOR>and insert blank lines - Add/update test suites under
tests/Separatorandtests/ImportsManualSeparatedto cover manual separators
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/constants.ts | Exported SEPARATOR_SPECIAL_WORD |
| src/utils/get-sorted-nodes-by-import-order.ts | Handle SEPARATOR_SPECIAL_WORD in the import-sorting logic |
| src/utils/tests/get-code-from-ast.spec.ts | Added : any annotation for the AST variable |
| tests/Separator/ppsi.spec.js | New spec invoking <SEPARATOR> in importOrder |
| tests/Separator/* | Various input files and snapshots updated for separator behavior |
| tests/ImportsManualSeparated/ppsi.spec.js | Manual-separated tests updated to include <SEPARATOR> |
| tests/ImportsManualSeparated/* | Additional manual separation scenarios and snapshots |
| @@ -0,0 +1,5 @@ | |||
| run_spec(__dirname, ["typescript"], { | |||
| importOrder: ['^@core/(.*)$', '^@server/(.*)', '^@ui/(.*)$', '<SEPARATOR>', '^[./]'], | |||
There was a problem hiding this comment.
The regex '^@server/(.)' is missing the trailing '$' to match the end of the string. For consistency with the other patterns, it should be '^@server/(.)$'.
| importOrder: ['^@core/(.*)$', '^@server/(.*)', '^@ui/(.*)$', '<SEPARATOR>', '^[./]'], | |
| importOrder: ['^@core/(.*)$', '^@server/(.*)$', '^@ui/(.*)$', '<SEPARATOR>', '^[./]'], |
| plugins: getExperimentalParserPlugins([]), | ||
| }; | ||
| const ast = babelParser(code, parserOptions); | ||
| const ast: any = babelParser(code, parserOptions); |
There was a problem hiding this comment.
[nitpick] Using any for the AST reduces type safety. Consider importing and using Babel's AST types (e.g., babelParser.ParseResult<File>) to improve maintainability.
| const ast: any = babelParser(code, parserOptions); | |
| const ast: babelParser.ParseResult<File> = babelParser(code, parserOptions); |
|
This pr is now obsolete with #339 merged now, so i think this can be safely closed. |
Implement feature #268