Hyper: Initial hyper session is not created correctly

Created on 27 Jul 2019  路  8Comments  路  Source: vercel/hyper

  • [x] I am on the latest Hyper.app version
  • [x] I have searched the issues of this repo and believe that this is not a duplicate
  • OS version and name: macOS 10.14.5
  • Hyper.app version: 3.0.2

Issue

Hi :) Thanks for your work on this!

I use https://github.com/hharnisc/hypercwd and noticed the following bug https://github.com/hharnisc/hypercwd/issues/71:

The problem is:
The initial working directory is not set correctly for the very first hyper session, but people noticed that after a full reload the CWD was set correctly again. The issue on hypercwd seems a bit stale and nobody seems to know how to fix it, so I took a little dive into the code and this is what I found:

First: the bug does not seem to come from the hypercwd codebase, but rather from this codebase.

The problem seems to be located in the optimistically/eagerly creating an initial session here: https://github.com/zeit/hyper/blob/c13c8a5c52cbc8e02098244193a4e715fffcd902/app/ui/window.js#L140
To be more exact in createInitialSession():
https://github.com/zeit/hyper/blob/c13c8a5c52cbc8e02098244193a4e715fffcd902/app/ui/window.js#L124

Here createSession() is optimistically called without passing any parameter, although the function is defined as createSession(extraOptions = {}). Those extraOptions are used to set the CWD (among other things correctly), see: https://github.com/zeit/hyper/blob/c13c8a5c52cbc8e02098244193a4e715fffcd902/app/ui/window.js#L111

    function createSession(extraOptions = {}) {
      const uid = uuid.v4();

      const defaultOptions = Object.assign(
        {
          rows: 40,
          cols: 100,
          cwd: process.argv[1] && isAbsolute(process.argv[1]) ? process.argv[1] : homeDirectory,
          splitDirection: undefined,
          shell: cfg.shell,
          shellArgs: cfg.shellArgs && Array.from(cfg.shellArgs)
        },
        extraOptions, // <-- this is the important line
        {uid}
      );
      const options = decorateSessionOptions(defaultOptions);
      const DecoratedSession = decorateSessionClass(Session);
      const session = new DecoratedSession(options);
      sessions.set(uid, session);
      return {session, options};
    }

This optimistically creating an initial session without passing the correct extraOptions is the underlying problem for the mentioned issue. The CWD is set incorrectly, because it is not set at all. After a full reload however it is then overwritten correctly.

I am unsure about the reasoning for this optimistic session creating and whether it is a technical necessity or a performance optimization.

I tried removing https://github.com/zeit/hyper/blob/canary/app/ui/window.js#L140 this line and everything seemed to still work and the bug was fixed.

I hope this analysis provides enough insight for you to come up with a decision on how to proceed quickly as the issue really is quite annoying.

Thanks for your help, have a nice day :)

Bug

Most helpful comment

@chabou Friendly ping :) Can you please take a look at this? It's a really annoying issue :/ Thanks!

All 8 comments

@chabou Friendly ping :) Can you please take a look at this? It's a really annoying issue :/ Thanks!

Any progress?
It's seems like an easy fix and it's really annoying.
Every extension relying on extraOptions is broken... @chabou

I would like to bump this issue as well. I also find this issue to be annoying when using hypercwd and other Hyper plugins. It seems like a fix should be simple enough since @stnwk has already done such stellar investigative work.

@ahmedx2

It seems like a fix should be simple enough since @stnwk has already done such stellar investigative work.

Thanks 鈽猴笍

Any of you want to create PR for this?

Happy to do so.

Will create a PR with the suggested solution I outlined in my initial analysis of this problem :)

Update: Sorry, I thought I'd have the resources to do this, but unfortunately currently I don't.

Happy if someone else would like to come up with a PR.
A possible solution was already described in the original post.

@Stanzilla @stnwk Done: #3937

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dbkaplun picture dbkaplun  路  3Comments

stan-stripe picture stan-stripe  路  3Comments

cilice picture cilice  路  3Comments

hxnt picture hxnt  路  3Comments

anthonyettinger picture anthonyettinger  路  3Comments