Skip to content

vimPlugins.peek-nvim: build JS#403252

Merged
GaetanLepage merged 1 commit intoNixOS:masterfrom
GaetanLepage:peek-nvim
May 2, 2025
Merged

vimPlugins.peek-nvim: build JS#403252
GaetanLepage merged 1 commit intoNixOS:masterfrom
GaetanLepage:peek-nvim

Conversation

@GaetanLepage
Copy link
Contributor

@GaetanLepage GaetanLepage commented May 1, 2025

Things done

Currently, using peek.nvim fails with:

Peek error: error: Module not found "file:///nix/store/a0xhznlqbcjw587yj88s73bpjs0sw048-vimplugin-peek.nvim-2024-04-0
9/public/main.bundle.js".
Press ENTER or type command to continue

We should be building the JS in the buildPhase.
Upstream explains that it should be:

deno task --quiet build:fase

but this requires internet access.
So instead, we should fetch the dependencies before hand.
I am not sure about how to do that though.

cc @MattSturgeon @Lurgrif

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: vim Advanced text editor label May 1, 2025
@nix-owners nix-owners bot requested review from PerchunPak, khaneliman and mrcjkb May 1, 2025 09:02
@GaetanLepage GaetanLepage marked this pull request as draft May 1, 2025 09:02
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 1, 2025
@MattSturgeon
Copy link
Contributor

MattSturgeon commented May 1, 2025

Looking upstream, it seems the package is described by deno.json. Looking at deno's docs, deps are listed in the "imports" object:

"imports": {
  "node:punycode": "https://deno.land/x/punycode/punycode.js"
},

Elsewhere in deno's docs, it is mentioned that dependency locks are configured by the "lock" object of deno.json; which upstream leaves undefined. The docs mention the default lockfile is ./deno.lock, however this isn't tracked in upstream's git repo. Therefore I believe we'll need to generate and track our own deno.lock in nixpkgs (or request that upstream start locking their dependencies).


I was hoping something relevant would be in https://nixos.org/manual/nixpkgs/unstable/#language-javascript, however I can't see any mention of deno.

Searching nixpkgs's github for "deno.lock" reveals no code matches, no matching issues, and only one relevant PR: #326003. That PR is a (slightly stale) draft for implementing a buildDenoApplication function. Maybe there are some interesting ideas in its implementation or review feedback?

EDIT: another related PR: #358252

@MattSturgeon
Copy link
Contributor

Looks like in addition to needing deno at compile time, it is also needs to be on the PATH at runtime...

Upstream runs its binary using deno too: https://github.com/toppair/peek.nvim/blob/5820d937d5414baea5f586dc2a3d912a74636e5b/lua/peek/app.lua#L40-L45

cmd = vim.list_extend({
  'deno',
  'task',
  '--quiet',
  'run',
}, args)

However it still needs to have been built; even if deno is on the PATH you currently still get an error:

Peek error: error: Module not found "file:///nix/store/5mbbz1v7rrn9a9ddvgjhjr25kjfialcp-vimplugin-peek.nvim-2024-04-09/public/main.bundle.js".

If deno isn't on the PATH at all, you get a different error:

Error executing Lua callback: Vim:E475: Invalid value for argument cmd: 'deno' is not executable
stack traceback:
        [C]: in function 'jobstart'
        ...r/pack/myNeovimPackages/start/peek.nvim/lua/peek/app.lua:53: in function 'init'
        .../pack/myNeovimPackages/start/peek.nvim/lua/peek/init.lua:23: in function 'open'
        .../pack/myNeovimPackages/start/peek.nvim/lua/peek/init.lua:113: in function <.../pack/myNeovimPackages/start/peek.nvim/lua/peek/init.lua:104>

@MattSturgeon
Copy link
Contributor

MattSturgeon commented May 1, 2025

@GaetanLepage I've made some progress on this; it's a little ugly, but we can patch how the plugin runs its cmd.

This probably isn't the correct approach, but it's hard to avoid patching when the plugin is hard-coded to run it's exe via deno in a very specific way 😕

Diff

diff --git a/pkgs/applications/editors/vim/plugins/overrides.nix b/pkgs/applications/editors/vim/plugins/overrides.nix
index 72db22346c62..8a87296b70e8 100644
--- a/pkgs/applications/editors/vim/plugins/overrides.nix
+++ b/pkgs/applications/editors/vim/plugins/overrides.nix
@@ -9,6 +9,7 @@
   fetchpatch,
   fetchurl,
   neovimUtils,
+  wrapNeovimUnstable,
   replaceVars,
   symlinkJoin,
   # Language dependencies
@@ -2744,17 +2745,28 @@ in
     ];
   };
 
-  peek-nvim = super.peek-nvim.overrideAttrs {
-    nativeBuildInputs = [ deno ];
+  peek-nvim = super.peek-nvim.overrideAttrs (old: {
 
-    buildPhase = ''
-      runHook preBuild
-
-      deno task --quiet build:fast
+    # Patch peek-nvim to run using nixpkgs deno
+    # This means end-users have to build peek-nvim the first time they use it...
+    patches = [
+      (replaceVars ./patches/peek-nvim/cmd.patch {
+        DENO = lib.getExe deno;
+      })
+    ];
 
-      runHook postBuild
-    '';
-  };
+    passthru = old.passthru // {
+      example = wrapNeovimUnstable neovim-unwrapped {
+        plugins = [ self.peek-nvim ];
+        luaRcContent = ''
+          require("peek").setup({})
+          vim.api.nvim_create_user_command("PeekOpen", require("peek").open, {})
+          vim.api.nvim_create_user_command("PeekClose", require("peek").close, {})
+        '';
+        # wrapperArgs = "--prefix PATH : ${lib.makeBinPath [ deno ]}";
+      };
+    };
+  });
 
   persisted-nvim = super.persisted-nvim.overrideAttrs {
     nvimSkipModules = [
diff --git a/pkgs/applications/editors/vim/plugins/patches/peek-nvim/cmd.patch b/pkgs/applications/editors/vim/plugins/patches/peek-nvim/cmd.patch
new file mode 100644
index 000000000000..1fd0a6c39b0b
--- /dev/null
+++ b/pkgs/applications/editors/vim/plugins/patches/peek-nvim/cmd.patch
@@ -0,0 +1,38 @@
+diff --git a/app/src/main.ts b/app/src/main.ts
+index c82d914..e8542f3 100644
+--- a/app/src/main.ts
++++ b/app/src/main.ts
+@@ -73,7 +73,7 @@ async function init(socket: WebSocket) {
+     const onListen: Deno.ServeOptions['onListen'] = ({ hostname, port }) => {
+       const serverUrl = `${hostname.replace('0.0.0.0', 'localhost')}:${port}`;
+       logger.info(`listening on ${serverUrl}`);
+-      const webview = new Deno.Command('deno', {
++      const webview = new Deno.Command('@DENO@', {
+         cwd: dirname(fromFileUrl(Deno.mainModule)),
+         args: [
+           'run',
+diff --git a/lua/peek/app.lua b/lua/peek/app.lua
+index af5148e..5e67563 100644
+--- a/lua/peek/app.lua
++++ b/lua/peek/app.lua
+@@ -38,10 +38,17 @@ function module.setup()
+   end
+ 
+   cmd = vim.list_extend({
+-    'deno',
+-    'task',
+-    '--quiet',
++    '@DENO@',
+     'run',
++    '--allow-read',
++    '--allow-write',
++    '--allow-net',
++    '--allow-env',
++    '--allow-run',
++    '--no-check',
++    '--allow-import',
++    '--no-lock',
++    '../../app/src/main.ts',
+   }, args)
+ end
+ 

passthru.example can be omitted from the PR, I'm using it locally to test.

This skips the build step entirely, instead running from source and downloading deps at runtime. This isn't ideal, but should at least work until we have a sane way to build it in nixpkgs 😕

@GaetanLepage
Copy link
Contributor Author

Thanks a lot @MattSturgeon! I included your latest patch and put you as a co-author.

@GaetanLepage GaetanLepage marked this pull request as ready for review May 2, 2025 15:19
@GaetanLepage
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 403252 --package vimPlugins.peek-nvim


x86_64-linux

✅ 1 package built:
  • vimPlugins.peek-nvim

aarch64-linux

✅ 1 package built:
  • vimPlugins.peek-nvim

x86_64-darwin

✅ 1 package built:
  • vimPlugins.peek-nvim

aarch64-darwin

✅ 1 package built:
  • vimPlugins.peek-nvim

@GaetanLepage GaetanLepage force-pushed the peek-nvim branch 2 times, most recently from 05c01de to 7e75da2 Compare May 2, 2025 16:37
@GaetanLepage GaetanLepage requested a review from MattSturgeon May 2, 2025 16:42
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

Need to check this actually works in practice, but the diff looks good.

Co-authored-by: Matt Sturgeon <matt@sturgeon.me.uk>
@GaetanLepage GaetanLepage merged commit 91bd4eb into NixOS:master May 2, 2025
22 checks passed
@GaetanLepage GaetanLepage deleted the peek-nvim branch May 2, 2025 21:05
@stelcodes
Copy link
Contributor

stelcodes commented May 6, 2025

I can't get this to work with my neovim setup. I call require("peek").open() and my browser opens with a blank page that says Not Found. The issue is that the value Deno.mainModule is expected to be public/main.bundle.js, not app/src/main.ts.

https://docs.deno.com/api/deno/~/Deno.mainModule

The server cannot locate the index.html required to load the page:

      const webview = new Deno.Command('deno', {
        cwd: dirname(fromFileUrl(Deno.mainModule)),
        args: [
          'run',
          '--quiet',
          '--allow-read',
          '--allow-write',
          '--allow-env',
          '--allow-net',
          '--allow-ffi',
          '--unstable',
          '--no-check',
          'webview.js',
          `--url=${new URL('index.html', Deno.mainModule).href}`,
          `--theme=${__args['theme']}`,
          `--serverUrl=${serverUrl}`,
        ],
        stdin: 'null',
      });
  async function findFile(url: string) {
    const path = new URL(url).pathname.replace(/^\//, '') || 'index.html';
    for (const base of [Deno.mainModule, 'file:']) {
      const openUrl = new URL(path, base)
      try {
        return await Deno.open(openUrl);
      } catch (_) { /**/ }
    }
  }

But beyond that issue, I don't get how this is going to work because the build step is still missing?

@MattSturgeon
Copy link
Contributor

I can't get this to work with my neovim setup. I call require("peek").open() and my browser opens with a blank page that says Not Found.

Thanks for testing!

The issue is that the value Deno.mainModule is expected to be public/main.bundle.js, not app/src/main.ts.

Am I understanding that the only issue is the URL is relative to app/src instead of public?

I would've expected the index.html file to be in /tmp or something 🤔

But beyond that issue, I don't get how this is going to work because the build step is still missing?

Is building actually necessary? Does it do anything beyond converting the typescript to javascript?

I don't see how we'll get a solution for building without something like #358252. I guess, as it's only one dependency, we could fetch that dependency manually and trick deno into thinking it's cached?

Personally, I'm unfamiliar with this plugin, and deno more generally.

@stelcodes
Copy link
Contributor

stelcodes commented May 6, 2025

The build step (done by deno task build:fast) does a lot, downloading a bunch of JS, CSS, and fonts as well as building 3 separate JS bundles (server, client, webkit). This package is not going to work with until we have proper support for handling Deno builds (#196124). You would also need to replace all the extra logic in scripts/build.ts with the Nix equivalent. I think this package should be marked as broken or removed in the meantime.

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

Labels

6.topic: vim Advanced text editor 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants