-
-
Notifications
You must be signed in to change notification settings - Fork 33
Add documentation for import maps feature #457
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
Conversation
Warning Rate limit exceeded@lambdalisue has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughDocumentation update to doc/denops.txt adding two new top-level sections—DEVELOPMENT and IMPORT MAPS—along with their entries in the CONTENTS. The IMPORT MAPS section details configuration, supported files, formats, usage, limitations, examples, and implementation notes. The same new sections are inserted twice in the file. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (9)
doc/denops.txt (9)
547-554
: Clarify scan scope to avoid confusion with repo-root configsExplicitly state that denops searches “the plugin directory (i.e. the directory containing main.ts)” and does not look outside it unless a workspace is set up, to prevent users expecting repo-root deno.json to apply automatically.
Apply this minimal addition after Line 553:
resolve module specifiers during plugin import. + +By default, denops only scans the plugin directory (the directory that contains +the plugin's "main.ts") and does not traverse parent directories. Use a Deno +workspace (see below) when you want to keep a repo‑root configuration.
572-582
: State “no upward traversal” and precedence explicitly in Supported FilesYou imply order, but making the non-traversal and precedence explicit reduces ambiguity.
The import map feature searches for the following files in order: @@ 4. `import_map.jsonc` (JSONC with comments support) -The first file found will be used as the import map for the plugin. +The first file found in the plugin directory will be used. Parent directories +are not searched, and no files outside the plugin directory are considered +unless a Deno workspace is configured.
585-597
: Add brief note on prefix mappings (trailing slash) to aid readers new to import mapsThis prevents common confusion about when to add a trailing slash.
Import map files should contain an "imports" field that maps module specifiers to their corresponding URLs: @@ "@test/helper": "./helper.ts" } } + +Note: A trailing slash in the specifier (e.g. `"@test/"`) defines a path +prefix mapping and will match any subpath under that prefix.
598-612
: JSONC snippet: small whitespace cleanupThere’s an extra trailing whitespace line after "@std/async". Remove to keep help file tidy.
- "@std/async": "jsr:@std/async@^1.0.1", - + "@std/async": "jsr:@std/async@^1.0.1",
614-633
: Usage example: add a quick comment indicating which imports are map-resolvedReaders may not realize which lines rely on the import map vs standard JSR resolution.
>typescript - import type { Denops } from "@denops/core"; - import { join } from "@std/path"; - import { delay } from "@std/async"; - import { helper } from "@test/helper"; + import type { Denops } from "@denops/core"; // from JSR + import { join } from "@std/path"; // via import map + import { delay } from "@std/async"; // via import map + import { helper } from "@test/helper"; // via import map (local)
634-638
: Limitations: soften “must be in same directory” or align with earlier descriptionEarlier you say “searches the plugin directory”; here “must be in the same directory as the main file” is stronger. Either keep the softer “plugin directory” wording or explicitly define “same directory” = plugin directory containing main.ts.
-- Import map files must be located in the same directory as the plugin's - main file. +- Import map files must be located in the plugin directory (i.e., the directory + that contains the plugin's "main.ts").
640-649
: Example structure: highlight which deno.json is used by denops vs workspaceMake it explicit that the plugin-local deno.json is used for import maps, while the root one is for workspace/tooling.
my-vim-plugin/ - ├── deno.json + ├── deno.json # (optional) repo-root; defines "workspace" for tooling └── denops/ └── my-plugin/ ├── main.ts ├── lib/ │ └── utils.ts - └── deno.json + └── deno.json # plugin-local; import maps read from here
651-656
: Avoid locking docs to an internal library nameMentioning a specific internal library invites doc drift if implementation changes. Recommend documenting behavior, not the internal dependency.
-- The feature uses the `@lambdalisue/import-map-importer` library internally -- Import maps are loaded automatically when the plugin is imported +- Denops automatically loads and applies the import map when the plugin is imported + (implementation details are subject to change without notice) - JSONC parsing is handled using Deno's standard library
555-571
: Workspace section: call out minimum Deno version and show multi-plugin example
- Deno’s
"workspace"
field was introduced in Deno 1.45.0; add a note so plugin authors know which minimum version is required (deno.com).- Expand the example to illustrate multiple plugins in the workspace.
Suggested edit:
-For projects with a `deno.json` or `deno.jsonc` file in the root directory, -you can use the "workspace" field to enable Deno development tools to work -across plugin directories while maintaining separate dependency configurations: +For projects with a `deno.json` or `deno.jsonc` file in the repository root, +you can use Deno’s `"workspace"` field to enable its development tools +across plugin directories while keeping dependency configurations separate. + +Note: Requires Deno ≥1.45.0, where `"workspace"` support was added. >json { "workspace": [ - "./denops/my-plugin" + "./denops/my-plugin", + "./denops/another-plugin" ] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
doc/denops.txt
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-09-14T17:09:30.174Z
Learnt from: Milly
PR: vim-denops/denops.vim#418
File: tests/denops/runtime/functions/plugin/check_type_test.ts:6-6
Timestamp: 2024-09-14T17:09:30.174Z
Learning: In this project, import paths prefixed with `/denops-testdata/` are defined in `deno.jsonc` via an import map, and these import paths are valid.
Applied to files:
doc/denops.txt
🪛 LanguageTool
doc/denops.txt
[grammar] ~13-~13: There might be a mistake here.
Context: ... AUTOCMD |denops-autocmd| DEVELOPMENT |denops-development| IMPORT MAPS |d...
(QB_NEW_EN)
[grammar] ~534-~534: There might be a mistake here.
Context: ...======================================== DEVELOPMENT denops-development T...
(QB_NEW_EN)
[grammar] ~535-~535: There might be a mistake here.
Context: ...============================ DEVELOPMENT denops-development This section provi...
(QB_NEW_EN)
[grammar] ~539-~539: There might be a mistake here.
Context: ...comprehensive development guides, visit: - Denops Document: https://vim-denops.gith...
(QB_NEW_EN)
[grammar] ~540-~540: There might be a mistake here.
Context: ...m-denops.github.io/denops-documentation/ - Wiki: https://github.com/vim-denops/deno...
(QB_NEW_EN)
[grammar] ~541-~541: There might be a mistake here.
Context: ...://github.com/vim-denops/denops.vim/wiki - API Reference: https://jsr.io/@denops/st...
(QB_NEW_EN)
[grammar] ~545-~545: There might be a mistake here.
Context: ...---------------------------- IMPORT MAPS denops-import-maps Denops supports im...
(QB_NEW_EN)
[grammar] ~548-~548: There might be a mistake here.
Context: ...ou to define module specifiers and their corresponding URLs, making dependency ma...
(QB_NEW_EN)
[grammar] ~552-~552: There might be a mistake here.
Context: ...ory. If found, the import map is used to resolve module specifiers during plugin ...
(QB_NEW_EN)
[grammar] ~557-~557: There might be a mistake here.
Context: ...deno.jsonc
file in the root directory, you can use the "workspace" field to ena...
(QB_NEW_EN)
[grammar] ~558-~558: There might be a mistake here.
Context: ...to enable Deno development tools to work across plugin directories while maintain...
(QB_NEW_EN)
[grammar] ~559-~559: There might be a mistake here.
Context: ...ning separate dependency configurations: >json { "workspace": [ "./denops...
(QB_NEW_EN)
[grammar] ~560-~560: There might be a mistake here.
Context: ...eparate dependency configurations: >json { "workspace": [ "./denops/my-pl...
(QB_NEW_EN)
[grammar] ~568-~568: There might be a mistake here.
Context: ... between multiple plugins while allowing unified development tooling (deno fmt
,...
(QB_NEW_EN)
[grammar] ~569-~569: There might be a mistake here.
Context: ...o fmt,
deno lint,
deno test`) to run from the project root. Supported Files ...
(QB_NEW_EN)
[grammar] ~585-~585: There might be a mistake here.
Context: ...tain an "imports" field that maps module specifiers to their corresponding URLs: ...
(QB_NEW_EN)
[grammar] ~586-~586: There might be a mistake here.
Context: ... specifiers to their corresponding URLs: >json { "imports": { "@std/path"...
(QB_NEW_EN)
[grammar] ~587-~587: There might be a mistake here.
Context: ...fiers to their corresponding URLs: >json { "imports": { "@std/path": "jsr...
(QB_NEW_EN)
[grammar] ~598-~598: There might be a mistake here.
Context: ...ports comments for better documentation: >jsonc { // Import map configuration ...
(QB_NEW_EN)
[grammar] ~599-~599: There might be a mistake here.
Context: ...omments for better documentation: >jsonc { // Import map configuration for MyP...
(QB_NEW_EN)
[grammar] ~616-~616: There might be a mistake here.
Context: ...ou can use the mapped specifiers in your plugin code: >typescript import type { ...
(QB_NEW_EN)
[grammar] ~617-~617: There might be a mistake here.
Context: ...e mapped specifiers in your plugin code: >typescript import type { Denops } from ...
(QB_NEW_EN)
[grammar] ~618-~618: There might be a mistake here.
Context: ...cifiers in your plugin code: >typescript import type { Denops } from "@denops/cor...
(QB_NEW_EN)
🔇 Additional comments (2)
doc/denops.txt (2)
13-14
: CONTENTS entries and help tags look correctBoth entries point to valid anchors defined later. Nice adherence to Vim help conventions.
533-657
: All CONTENTS entries and anchors are unique and correctly matchedQuick counts confirm:
- “DEVELOPMENT” (
*denops-development*
) appears exactly once as a header (line 535) and once in the CONTENTS (line 13).- “IMPORT MAPS” (
*denops-import-maps*
) appears exactly once as a header (line 545) and once in the CONTENTS (line 14).No duplicate sections or tags detected.
- Add DEVELOPMENT section to denops.txt for plugin developers - Document import maps functionality including: - Workspace configuration for multi-plugin projects - Supported file formats (deno.json, import_map.json, etc.) - File format examples with JSON and JSONC - Usage examples in TypeScript plugins - Proper repository structure examples - Position development documentation at the end for better organization 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
a1d3b0e
to
7004eb9
Compare
Summary
denops.txt
for plugin developersDetails
This PR adds documentation for the recently implemented import maps feature. The documentation covers:
Import Maps Features
deno.json
workspace field to prevent configuration conflictsdeno.json
,deno.jsonc
,import_map.json
,import_map.jsonc
)denops/
directoryDocumentation Organization
Test plan
:help denops-development
and:help denops-import-maps
🤖 Generated with Claude Code
Summary by CodeRabbit