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

Add section on VSCode folder-/watcher-ignore settings to "Project Configuration" docs (using .yarn-folder as an example) #5868

Merged
merged 6 commits into from
Aug 16, 2022

Conversation

Philzen
Copy link
Contributor

@Philzen Philzen commented Jul 3, 2022

Make VSCode ignore the .yarn-folder and thus exclude it from parsing.

Some more zen for the DX – and i guess some cpu cycles saved as VSCode won't try to discover and parse watch and search a 2.1MB javascript executable.

In the least it doesn't hurt to exclude this from the IDE – unless someone knows and actual need to do something in the .yarn-folder via VSCode.

@netlify
Copy link

netlify bot commented Jul 3, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit e8be9e4
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/62fb8bff1d8ded000837252f

@jtoar
Copy link
Contributor

jtoar commented Jul 7, 2022

Hey @Philzen. I like the idea of excluding the yarn binary from search, but removing the .yarn directory from the explorer seems a little too extreme. Is there a way we could just stop VSCode from searching it? Or what would be a good compromise?

@jtoar jtoar self-assigned this Jul 7, 2022
@Philzen
Copy link
Contributor Author

Philzen commented Jul 7, 2022

@jtoar would need some investigation on that. But ask yourself:

Did you ever in your life have to go into the .yarn-folder and do anything in there in VSCode? This is all there is:

grafik

I couldn't think of any scenario where i'd need that, for me it just clutters the DX, but maybe you know a actual use case for it being there.

For me, it's a bazillion times more likely that i'd want to go into the .git-folder and change some things in the config or write a custom hook – but that's also hidden. But if i need to work on it, i know it's there and i know my ways around 😉

@jtoar jtoar added topic/crwa create-redwood-app release:feature This PR introduces a new feature labels Jul 8, 2022
@jtoar
Copy link
Contributor

jtoar commented Jul 8, 2022

Did you ever in your life have to go into the .yarn-folder and do anything in there in VSCode?

I've used yarn patch before which generates files in .yarn/patches, but to be fair, I don't use VSCode to edit them—just to view them:

image

My concern is that this sounds like it count as adding magic to the framework. We show other dot directories that you're not really supposed to edit (but can for educational or debugging purposes). Namely, .redwood—everything in there is generated.

For me, it's a bazillion times more likely that i'd want to go into the .git-folder and change some things in the config or write a custom hook – but that's also hidden.

That's a fair point! But VSCode hides the .git directory automatically right? That one feels more like a well-established convention.

I appreciate the effort towards improving the DX, no matter how little, but on this particular change, I'm still unconvinced. So let's see if we can get some others to chime in.

@Philzen
Copy link
Contributor Author

Philzen commented Jul 10, 2022

My concern is that this sounds like it count as adding magic to the framework.

IMHO the whole RedwoodJS stack is magic ;)
But i do understand your point.

Is there a way we could just stop VSCode from searching it?

I have briefly looked into various posts on the question earlier (there's numerous on that) but none seemed to give a crystal clear answer on how to do that and i felt it would require some thorough testing to find the right config (provided it's actually possible).
Not sure if using "files.watcherExclude" is sufficient (otherwise i guess they could have mentioned that it also disables parsing for intellisense).

I've used yarn patch before which generates files in .yarn/patches, but [...] don't use VSCode to edit them—just to view them

Not sure if that was the most convincing argument to keep it in the directory listing 😆 – but surely honest though 🤝
Is it possible you may be having your RedwoodJS framework developer's hat on when you look into that folder? Which would make sense, b/c the yarn tooling is such an integral part of it.

I doubt anybody else – the consumers of RedwoodJS – ever really want to see that folder – they only care that it works and we provide them with a stable setup. Should they ever want to look into it – well there a plenty of options, and just like you would use those when intending to work inside the .git-folder they are available to do that. Or they could unhide it in the VSCode-Settings.

We show other dot directories that you're not really supposed to edit [...]. Namely, .redwood—everything in there is generated

Yes, but that one is really important to have imho – because VSCode navigates there all the time when a i click on a type to look up it's definition, so there is no doubt it is a real benefit to have it available.

With all of the above said i don't hold a very strong opinion on this topic or want to get my PR through at all costs – as a user i have already done that setting, never regretted it and simply wanted to share it – though i'd prefer not having to repeat it everytime i create a new redwood project 😉

I'm looking forward to more opinions on this though.

@jtoar
Copy link
Contributor

jtoar commented Jul 11, 2022

...as a user i have already done that setting, never regretted it and simply wanted to share it

For sure! And much appreciated, thanks for sharing—it's definitely hard for me to take my rwjs-framework-developer hat off as you said, and my "is this going to move the needle" hat off. I'll try to get more of the team's eyes on this!

@nx-cloud
Copy link

nx-cloud bot commented Jul 26, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit e8be9e4. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 14 targets

Sent with 💌 from NxCloud.

@jtoar
Copy link
Contributor

jtoar commented Jul 28, 2022

@Philzen I brought this up today with @Tobbe and we just don't think it's a good idea to make changes to the template if it's not a clear win / the majority of the community doesn't desperately need it. But we thought it may be a good idea to start collecting opt-in DX configurations like this though. Would you want to add a section to https://redwoodjs.com/docs/project-configuration-dev-test-build that talks about configuring your project?

@Philzen
Copy link
Contributor Author

Philzen commented Aug 9, 2022

I finally got around to write the requested section. This actually required some deep-dive into how config inheritance and files.exclude vs. search.exclude vs. files.watcherExclude works.

Learned along the way (by experimentation) that VSCode doesn't try to really parse the .yarn-folder through a lexer at all, that seems to be fully determined through {ts|js}config.json (which doesn't include it anyway).

Realized the other day that i can simply have it my way via a global setting in my user settings.json so i'll never have to bother again freshly created Redwood projects – so i can agree with the teams decision very easily, as i can now have my cake and also eat it ;)

@jtoar @Tobbe Kindly review and request any changes to the wording as you see fit. I completely fine with anybody pushing updates directly to improve upon this, as i'll be off for a couple of days from Thursday on, but would also love to get this finally merged soon (another tick in the box).

@Philzen Philzen changed the title Exclude yarn executable dir form search & parsing Add section on VSCode folder-/watcher-ignore settings to "Project Configuration" docs (using .yarn-folder as an example) Aug 9, 2022
@Philzen
Copy link
Contributor Author

Philzen commented Aug 10, 2022

@Tobbe / @jtoar:

  • change label to release/docs

@Tobbe Tobbe added topic/crwa create-redwood-app release:feature This PR introduces a new feature release:docs This PR only updates docs and removed release:feature This PR introduces a new feature topic/crwa create-redwood-app labels Aug 10, 2022
Copy link
Member

@Tobbe Tobbe left a comment

Choose a reason for hiding this comment

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

Really appreciate all the effort you put into this @Philzen. Reading it all, I think I'm probably going to enable this for my RW app projects 👍 Just had a couple of comments for you to look at

Philzen and others added 3 commits August 10, 2022 18:04
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
  - fix a typo
  - add command key combination for Mac
  - Reorder the ordered list so it matches the following subheadings order
@Tobbe
Copy link
Member

Tobbe commented Aug 10, 2022

Looks good to me, but let's give @jtoar some time to review if he wants. He's got a way with words that I lack 🙂

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

Great stuff @Philzen! Making some small edits I'll commit then reread again; I'll get this one in shortly.

docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
docs/docs/project-configuration-dev-test-build.mdx Outdated Show resolved Hide resolved
@jtoar jtoar enabled auto-merge (squash) August 16, 2022 12:22
@jtoar jtoar merged commit fcf6087 into redwoodjs:main Aug 16, 2022
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Aug 16, 2022
@jtoar jtoar modified the milestones: next-release, v3.0.0 Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:docs This PR only updates docs
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants