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.

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

configuration.ts, notation.ts circular dependencies #2275

Closed
Nodman opened this issue Jan 10, 2018 · 1 comment
Closed

configuration.ts, notation.ts circular dependencies #2275

Nodman opened this issue Jan 10, 2018 · 1 comment

Comments

@Nodman
Copy link
Contributor

Nodman commented Jan 10, 2018

Is this a BUG REPORT or FEATURE REQUEST?:
BUG REPORT

What happened:
I was trying to build master with VSCode 'build, run extension' profile but build failed due to:

Here is the error stack:  TypeError: Cannot read property 'leader' of undefined
extensionHostProcess.js:292
	at Function.NormalizeKey (/Users/spooner/Documents/temp/Vim/out/src/configuration/notation.js:23:49)
	at __dirname.reload.remapping.before.forEach (/Users/spooner/Documents/temp/Vim/out/src/configuration/configuration.js:230:105)
	at Array.forEach (native)
	at ConfigurationClass.reload (/Users/spooner/Documents/temp/Vim/out/src/configuration/configuration.js:230:38)
	at new ConfigurationClass (/Users/spooner/Documents/temp/Vim/out/src/configuration/configuration.js:202:14)
	at Object.<anonymous> (/Users/spooner/Documents/temp/Vim/out/src/configuration/configuration.js:360:25)
	at Object.<anonymous> (/Users/spooner/Documents/temp/Vim/out/src/configuration/configuration.js:362:3)
	at Module._compile (module.js:571:32)
	at Object.Module._extensions..js (module.js:580:10)
	at Module.load (module.js:488:32)

It looks like the problem is with circular dependencies between configuration.ts and notation.ts:

src/configuration/notation.ts:

...
import { Configuration } from './configuration';

export class Notation {
...
}

src/configuration/configuration.ts:

...
import { Notation } from './notation';
...
class ConfigurationClass {
...
}
...

export let Configuration = new ConfigurationClass();

notation.ts uses Configuration instance only to resolve "<leader> keymap":

if (key.toLocaleLowerCase() === '<leader>') {
  return Configuration.leader;
}

and at this point module is empty, so Configuration is undefined

What did you expect to happen:
success build

How to reproduce it (as minimally and precisely as possible):
add "<leader>" into user config for any keybinding, f.e:

{
  "before": [
    "<leader>",
    "<space>"
  ],
  "after": [],
  "commands": [
    {
      "command": "workbench.action.showCommands",
      "args": []
    }
  ]
}

and run build or tests from VSCode

Environment:

  • Extension (VsCodeVim) version: latest master branch (63be67e)
  • VSCode version: 1.19.1
  • OS version: macOS High Sierra (10.13.2)

I could submit PR if this is really an issue and I am not missing something and if someone helps me with the best solution to fix it as I'm not sure about it :/

Thanks!

@jpoon
Copy link
Member

jpoon commented Jan 10, 2018

Thanks for filing this issue. I was refactoring this code a couple of days ago and came across this. Typescript wasn't complaining about this circular dependency so I left it. Should be an easy fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants