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

Proxied types are not loaded. #26921

Closed
mjbvz opened this issue Sep 5, 2018 · 11 comments
Closed

Proxied types are not loaded. #26921

mjbvz opened this issue Sep 5, 2018 · 11 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Sep 5, 2018

From @zavr-1 on September 4, 2018 3:46

Issue Type: Bug

When a package loads some modules via another module, their will lack Intellisense and it is not possible to click through to their definition.

VS Code version: Code - Insiders 1.27.0-insider (346018a, 2018-08-24T05:12:36.061Z)
OS version: Darwin x64 15.6.0

System Info
Item Value
CPUs Intel(R) Core(TM)2 Duo CPU P7350 @ 2.00GHz (2 x 2000)
GPU Status 2d_canvas: unavailable_software
checker_imaging: disabled_off
flash_3d: unavailable_software
flash_stage3d: unavailable_software
flash_stage3d_baseline: unavailable_software
gpu_compositing: unavailable_software
multiple_raster_threads: disabled_off
native_gpu_memory_buffers: unavailable_software
rasterization: unavailable_software
video_decode: unavailable_software
video_encode: unavailable_software
webgl: unavailable_off
webgl2: unavailable_off
Load (avg) 7, 8, 6
Memory (System) 8.00GB (0.16GB free)
Process Argv /Applications/Visual Studio Code - Insiders.app/Contents/MacOS/Electron -psn_0_24582
Screen Reader no
VM 0%
Extensions (2)
Extension Author (truncated) Version
vscode-eslint dba 1.5.0
code-spell-checker str 1.6.10

from file A

import { throws, equal } from 'zoroaster/assert'

where zoroaster/assert is a proxy file (only has module.exports = require(./build/assert)) to zoroaster/build/assert which has:

let throws = require('assert-throws'); if (throws && throws.__esModule) throws = throws.default;
const $assert = require('assert');
const $assert_diff = require('assert-diff');



module.exports.assert = $assert
module.exports.equal = $assert.equal
module.exports.ok = $assert.ok
module.exports.deepEqual = $assert_diff.deepEqual
module.exports.throws = throws
//# sourceMappingURL=index.js.map
1. git clone https://github.com/artdecocode/vscode-require-types.git
2. code-insiders vscode-require-types
  1. open test/spec/default.js
  2. try use throws function, there will be no intellisense. try click through -- it will get to the parent package, but not any further than that.

test

  1. Uncomment // import assertThrows from 'assert-throws' line. Now, throws works fine.

test

Whenever the proxy in form of zoroaster/assert is removed in favour of zoroaster/build/assert, then things work. I suppose I have to ln the assert rather than use module.exports = require('./build/assert') but it's still a bug.

Copied from original issue: microsoft/vscode#57833

@mjbvz mjbvz self-assigned this Sep 5, 2018
@mjbvz mjbvz added the insiders label Sep 5, 2018
@mjbvz mjbvz removed their assignment Sep 5, 2018
@mjbvz mjbvz removed the insiders label Sep 5, 2018
@mjbvz
Copy link
Contributor Author

mjbvz commented Sep 5, 2018

The issue appears to be that that require('assert-throws') does not get resolved in ./node_modules/zoroaster/build/assert/index.js.

Adding a d.ts typings for zoroaster should fix this as well

@zavr-1
Copy link

zavr-1 commented Sep 6, 2018

I don't like TypeScript, I don't think it solves any problems because JS is a duck typed language and using JSDoc with a proper IDE like VS Code which can understand @ type {import(.)} Type should be enough. The problem however is more here because I found another case where it happened.

ALSO the require('assert-throws') does get resolve properly, if you import it directly via import { throws } from 'zoroaster/build/assert, or when the proxy assert file is lned. I think the problem is that TypeScript parser does not go more than 1 level deep, but I'll double check that.

@mjbvz
Copy link
Contributor Author

mjbvz commented Sep 6, 2018

TypeScript powers both VS Code's javascript and typescript intellisense. It is what analyzes jsdocs

I recommend that you try writing d.ts typings for zoroaster. Even if this specific issue is fixed, proper typings will provide a much better experience

@zavr-1
Copy link

zavr-1 commented Sep 6, 2018

Thanks but I explained that I will never write a typings file, I am more than happy with JSDoc and not being part of the whole TS movement. Maybe it's hard to understand but there're true JS fans out there. And why do you think it's a much better experience when experience is literally the same when proper JSDoc is written? Like honestly could you give me an example of "better experience"? Before @type {import('.')} Type in JSDoc it might have been true, but not anymore. There's no need to write and publish typings when JS is not for types (apart from auto-completion, which is once again is achieved with JSDoc).

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Sep 21, 2018
@RyanCavanaugh
Copy link
Member

This works fine for me

image

@zavr-1
Copy link

zavr-1 commented Sep 25, 2018

@RyanCavanaugh This is not what it supposed to be showing, the signature of the throws function is different. The problem is quite annoying actually see

bug

In the end when koa-router gets selected it's me clicking with cmd trying to jump to that file, but it does not work. You can try in this repo in example/Session. Or make a project, add @idio/core and try to make a server with it.

import idioCore from '@idio/core'

const Server = async () => {
  const { url } = await idioCore()
  console.log(url)
}

It's dodgy because the type for Application which is just import of koa is showing fine, but for router not, and for middleware is not, because it's imported from another module.

bug2

And it also works in the project directory, with the src folder, but the problem starts when the build is done. Therefore there is a need to just return an object with a known type because a constructor was used, but it does not work because the type is not proxied. It works for packages and files without - but not with. If I open the workspace initially with that file open, it will initialise fine, but not when I then close or jump to it. That's the summary.

@zavr-1
Copy link

zavr-1 commented Sep 25, 2018

CAN YOU FIX THIS PLEASE IT IS THE MOST ANNOYING BUG EVER

STEPS TO REPRODUCE

  1. git clone [email protected]:idiocc/test.git
  2. code test
  3. Open src/index.js, hover over idio.
  4. Click through on idio so that build/index.js opens.
  5. Click through on startApp so that build/lib/start-app.js opens.
  6. Go up to the heading of the file, try clicking on require('./setup-middleware'). This does not work!!! Types from there are not loaded!

This is VS Code

screen shot 2018-09-25 at 20 58 05

This is VS Code Insiders

screen shot 2018-09-25 at 20 58 50

In Code at least I get a Router (without Middleware, but with Application coming up weirdly), in Insiders I don't get neither router nor middleware because of #27302! It's a disaster!

@weswigham
Copy link
Member

clickclickclick

Am I doing something from your repro wrong?

@zavr-1
Copy link

zavr-1 commented Sep 26, 2018

@weswigham OK thanks very much for testing, can you basically make sure that src/index.js stays open when you do that by double clicking on it?

@zavr-1
Copy link

zavr-1 commented Oct 1, 2018

@weswigham @RyanCavanaugh I beg you can you make this work it's making all the efforts of writing documentation worthless look it happens everywhere after 1 level of require:

  1. git clone https://github.com/rqt/test2.git (https://github.com/rqt/test2.git)

  2. cd test2

  3. npm i / yarn

  4. open src/index.js so that it's always open (no italic)

  5. add .create to gh.repos -> there will be no suggestions!!! This is the bug!!! Also try clicking though on import @rqt/github then on ./api/repos on top at let repos = require('./api/repos'); then you won't be able to click through to ./create!
    screen shot 2018-10-01 at 16 48 43

  6. navigate to node_modules/rqt/github/build/api/repos/ and double click on it to always open (no italic)

  7. go back to src/index and there will be suggestions! But this is only because the file is open in the workspace.
    screen shot 2018-10-01 at 16 48 27

I believe Microsoft should show respect to people who write in JS and choose JSDoc as their types engine without TypeScript. This is a really important bug but nobody's noticed it because everyone around choose to write types definitions. Well I don't want to do that it's not fair to treat me like an outcast and force to convert to TS!

@RyanCavanaugh
Copy link
Member

I haven't seen other reports of whatever this is, so either we've fixed it or it's rare enough to not warrant further investment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

4 participants