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

VS Code features not working in RW (b/c of imports) #896

Closed
Tobbe opened this issue Jul 27, 2020 · 8 comments
Closed

VS Code features not working in RW (b/c of imports) #896

Tobbe opened this issue Jul 27, 2020 · 8 comments
Labels
bug/confirmed We have confirmed this is a bug topic/fully-integrated-dx

Comments

@Tobbe
Copy link
Member

Tobbe commented Jul 27, 2020

Because of the way RW does imports there are a few useful features of VS Code that aren't working.

Go to Definition

"Go to Definition" is triggered by F12 or Ctrl+Click in VS Code. The way it should work is, if I Ctrl+Click on a component it should open up the file with the source code for that component. With the way Redwood does imports that doesn't work.

Current behavior

I start with this
image

I then Ctrl+Click on <MainLayout> and the highlight jumps to the import at the top of the file

image

Ctrl+Click on the import gives me this

image

This isn't very useful

Expected behavior

I start with this (notice that I changed to a relative import)

image

Ctrl+Click on <MainLayout> and I'm taken to this file instead

image

Hover

When hovering a component VS Code should show the "signature" of that component (i.e. telling you what props it takes, what it returns, etc)

Current behavior

image

Expected behavior

image

Ctrl+Hover

This is the combination of the two features above

Current behavior

image

Expected behavior

image

@aldonline
Copy link
Contributor

In order for VSCode to understand the "src/x/y/z" import specifiers, one option is to include a jsconfig.json (or tsconfig.json) that sets the "paths" compiler option. Something like this:

{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "src/*":["src/*"]
    }
  }
}

However, given how much "redwood magic" is going on (with "directory named" convention, automatic imports and the importAll macro),
I'm not sure how this interacts with everything else. This is something that needs to be solved holistically, so I defer to @peterp for the final answer.

@Tobbe
Copy link
Member Author

Tobbe commented Aug 7, 2020

Another thing that's not working because of how RW does imports is JSDocs

With a relative import I get this on hover
image

With RW imports all I get is this
image

Given how many awesome/helpful VSCode features we're missing out on to have "nicer" imports - is it really worth it?

In the TS projects at work I never manually write any imports anymore, I just let VSCode autoimport whatever I need. So it doesn't matter if it's ../../../ or ../../../../, I never have to care about that anyway. VSCode does it for me.

@peterp
Copy link
Contributor

peterp commented Aug 8, 2020

Hey Tobbe,

This type of DX is an area that really matters to us, so thanks for bringing this up, we're going to fix all of these.

There are really three magical things at play here. I'll explain how they work and give guidance on how one can opt-out if you don't like the behaviour, but I fully expect to make the experience "VSCode compliant" in the near future. Along with the TypeScript work that I'm currently doing.

Module Alias

  1. We introduce an alias src that maps against ./api/src/ and ./web/src:
import { db } from 'src/lib/db'
// -> ./api/src/lib/db

@Tobbe I do not know why you're not seeing JSDoc and type information, this is working for me with a standard "create redwood app". Do you have a jsconfig.json or tsconfig.json that specifies that alias in paths?

image

Opting out

Remove the "paths" part from your {js,ts}config.json and VSCode should be able to determine the relative imports again.

I believe the amount of places where we reference the src/ alias is only in api/src/functions/graphql.js

Directory named imports

  1. We use directory named imports.
import MyHeader from 'src/components/MyHeader' 
// -> ./web/src/components/MyHeader/MyHeader.js

This is a webpack plugin, we have an open issue to convert it to babel, which will fix a lot of the issues mentioned here.

This only applies to the web side.

Opting out

Rename your component from MyHeader/MyHeader.js to MyHeader/index.js

Auto global imports

We use another babel plugin that detects when you're trying to use gql, React or PropTypes

When you use one of those we automatically import them for you. We have just very recently shipped a PR that adds global TypeScript definitions for those.

image

Opting out

Pretend they don't exist and import them yourself. The plugin is intelligent enough to not import it twice.

ImportAll/ Import *

These will go away soon. It's an implementation detail that doesn't add any value to the consumer of RWJS.

@Tobbe
Copy link
Member Author

Tobbe commented Aug 8, 2020

Thanks for the detailed response 👍

This type of DX is an area that really matters to us, so thanks for bringing this up, we're going to fix all of these.

❤️

@Tobbe I do not know why you're not seeing JSDoc and type information, this is working for me with a standard "create redwood app". Do you have a jsconfig.json or tsconfig.json that specifies that alias in paths?

That screenshot I posted was from the RW framework project. I haven't touched any of the {j,t}sconfnig.json files. But I just went and looked, and it does not seem like these aliases are specified.

We use another babel plugin that detects when you're trying to use gql, React or PropTypes

This is really nice!

@jtoar jtoar added bug/confirmed We have confirmed this is a bug v1/priority labels Dec 10, 2021
@jtoar jtoar added next and removed v1/priority labels Dec 21, 2021
@jtoar jtoar removed the next label May 5, 2022
@Philzen
Copy link
Contributor

Philzen commented Aug 6, 2022

@Tobbe This is also something that has been nagging me every now and then as well. It's strange, b/c sometimes it works fine, sometimes CTRL-Click just makes VSCode jump to the import statement instead of the component implementation just as you described.

From my observations, using an absolute import such as src/layouts/MainLayout/MainLayout (referencing the file instead of only the component directory) always works (such as the relative import), doesn't it?

Related issues:

@Tobbe
Copy link
Member Author

Tobbe commented Aug 7, 2022

I hadn't seen those other issues, thanks for linking.

If getting rid of directory named imports solves this then I'm all for getting rid of them. But the pushback from the rest of the team is probably going to be pretty strong. So we need to build a very solid case to get this one through.

I think we need:

  • A clear demonstration of what's not working right now that's reproducible on multiple machines
  • A PR that removes the directory named imports and instead uses full imports MainLayout/MainLayout
  • Another clear demonstration showing all the awesome features we get access to with full imports
    • I know an early argument for using directory named imports was so you wouldn't have to put an index.js file in every single directory. So we also need to show that no index file is needed.

Do you want to help with that?

@Josh-Walker-GM
Copy link
Collaborator

@Tobbe I just ran through your example at the top and in the second comment. I could see the expected behaviour for all of these in a new 6.3.1 project. I'm not sure exactly when these would have been resolved. I also know there are likely other issues like this one that aren't resolved. Thoughts on closing this one?

@Tobbe
Copy link
Member Author

Tobbe commented Oct 21, 2023

Confirmed. Works for me too!

@Tobbe Tobbe closed this as completed Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug topic/fully-integrated-dx
Projects
Status: Done
Development

No branches or pull requests

6 participants