Dvc tree #53

RandomFractals · 2021-01-02T21:48:21Z

Add basic file tree view for #45

for files tree view

- fix extension icon and info fields in package.json

shcheklein · 2021-01-02T21:52:53Z

@RandomFractals I think @rogermparent fixed the yarn install in the root to install all the hooks and it should run prettier, ts linters, etc. Please, could you please update and run the again? It looks like style is off at the moment. (@rogermparent let's prioritize #23 please? it makes it hard to collaborate w/o those checks).

shcheklein · 2021-01-02T21:58:32Z

@RandomFractals could you please briefly describe what is exactly does at the moment? (it can wait until Monday for sure).

RandomFractals · 2021-01-02T22:05:39Z

These changes set typical constants, file and string utils most vscode extensions have for us to build up.

The /tree folder contains implementation of a basic file tree for the dvc panel in activity bar based on vscode tree view example.

This is what it looks like:

Basically, it displays all files and folders with proper file icons like the built-in vscode file explorer and opens files on click. This should be a good start for us to start removing and adding files we'd like to see in DVC sidebar.

Next step is to add virtual tree nodes for missing data files, their revisions etc. based on dvc config files.

I am not sure why build fails. Error looks unrelated to my changes. Maybe @rogermparent can check it out since I am not really familiar with your current build process and it's very different from typical vscode extensions build.

shcheklein · 2021-01-02T22:13:43Z

@RandomFractals

it has quite a lot of errors, this might help?:

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB). This can impact web performance. Assets: main.js (288 KiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
main (288 KiB)
main.js

WARNING in webpack performance recommendations:
You can limit the size of your bundles by using import() or require.ensure to lazy load some parts of your application.
For more info visit https://webpack.js.org/guides/code-splitting/

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx(3,31):
TS2307: Cannot find module 'dvc-integration/src/DvcReader' or its corresponding type declarations.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx(225,21):
TS2339: Property 'params' does not exist on type 'DVCExperimentRow'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx(225,46):
TS2339: Property 'params' does not exist on type 'DVCExperimentRow'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx(226,22):
TS2339: Property 'metrics' does not exist on type 'DVCExperimentRow'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/build-dynamic-columns.tsx(226,49):
TS2339: Property 'metrics' does not exist on type 'DVCExperimentRow'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx(6,8):
TS2307: Cannot find module 'dvc-integration/src/DvcReader' or its corresponding type declarations.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx(89,20):
TS2339: Property 'baseline' does not exist on type 'unknown'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx(89,33):
TS2700: Rest types may only be created from object types.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/components/Experiments.tsx(188,37):
TS2339: Property 'queued' does not exist on type 'DVCExperimentRow'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/model/Model.ts
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/model/Model.ts(5,8):
TS2307: Cannot find module 'dvc-integration/src/webviewContract' or its corresponding type declarations.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/model/Model.ts
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/model/Model.ts(8,46):
TS2307: Cannot find module 'dvc-integration/src/DvcReader' or its corresponding type declarations.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/model/Model.ts
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/model/Model.ts(87,15):
TS2322: Type 'any' is not assignable to type 'never'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/util/build-experiment-tree.test.ts
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/util/build-experiment-tree.test.ts(107,47):
TS2345: Argument of type '{ queued: boolean; name: string; sha: string; subRows: ({ checkpoint_tip: string; queued: boolean; name: string; checkpoint_parent: string; sha: string; } | { checkpoint_tip: string; queued: boolean; checkpoint_parent: string; sha: string; name?: undefined; } | { ...; })[]; }' is not assignable to parameter of type 'DVCExperimentRow'.
Types of property 'subRows' are incompatible.
Type '({ checkpoint_tip: string; queued: boolean; name: string; checkpoint_parent: string; sha: string; } | { checkpoint_tip: string; queued: boolean; checkpoint_parent: string; sha: string; name?: undefined; } | { ...; })[]' is not assignable to type 'DVCExperimentRow[]'.
Type '{ checkpoint_tip: string; queued: boolean; name: string; checkpoint_parent: string; sha: string; } | { checkpoint_tip: string; queued: boolean; checkpoint_parent: string; sha: string; name?: undefined; } | { ...; }' is not assignable to type 'DVCExperimentRow'.
Type '{ checkpoint_tip: string; queued: boolean; name: string; checkpoint_parent: string; sha: string; }' has no properties in common with type 'DVCExperimentRow'.

ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/util/build-experiment-tree.ts
ERROR in /home/runner/work/vscode-dvc/vscode-dvc/webview/src/util/build-experiment-tree.ts(3,72):
TS2339: Property 'sha' does not exist on type 'DVCExperimentRow'.
Child html-webpack-plugin for "index.html":

The /tree folder contains implementation of a basic file tree for the dvc panel in activity bar based on vscode tree view example.

I would start with DVC-tracked files first. I'm not sure we want to replicate the whole tree from the very beginning. The most important now is to learn how to hook virtual files/directories. We can always get regular files around after that?

RandomFractals · 2021-01-02T23:22:11Z

yeah, I am not sure what those errors are. As you might have noticed they are for the webview classes, and I simply grabbed what was in the main branch.

as for the dvc file tree, I started with basic file tree because we'll need folders to find dvc tracked files. Listing those is next as I have mentioned above.

shcheklein · 2021-01-02T23:57:24Z

@RandomFractals it might be some changes to the package.json cause this (e.g. dvc-integration/src/DvcReader is kinda a signal of this, it looks like you did some renaming?). Do linters and build locally for you?

as for the dvc file tree, I started with basic file tree because we'll need folders to find dvc tracked files. Listing those is next as I have mentioned above.

it's a good point. But we might rely on the DVC itself first to get us list of files (as we do with experiments for example, instead of parsing them on our own). Otherwise we'll have to write our own parser for DVC metafiles, kinda start implementing DVC in JS. It can be done as an optimization later to my mind.

RandomFractals · 2021-01-03T15:46:12Z

@shcheklein yes, I changed extension name to dvc in package.json. Will check if that's what causing it.

I think that should be either dvc or vscode-dvc to match the repo name, and not dvc-itegration that is a remnant of the old scaffold.

I also think it would be best for us to add a more extensive Dev Build section to docs that lists the steps to pull, build, format, lint and commit code changes. I see a lot of emphasis on the latter part that I don't quite agree with, especially when some of those tools makes changes to the code being committed. Nothing against formatters and linters, but given the current state of extension.ts and other parts of this vscode extension I would like to see us focus more on building a good dvc commands foundation and split up and refactor some of those parts than get into prolonged linters discusssions. So, I might need to lift some of your linter rules and we can discuss it later. I think it's good to have some code rules, but we also need to have some quality code to back it up. Currently, this vscode extension is very loose in that aspect. We can always review and tighten formatting and linting once we have some basic things working as expected.

RandomFractals · 2021-01-03T16:24:11Z

ok. I ran ext. build and package scripts and was able to create vsix and install it in vscode.

So, this build issue must be upstream ...

Output from my local dvc ext. build:

Updates all but webpack libraries for this extension to use the latest vscode apis, etc.

shcheklein · 2021-01-03T17:18:10Z

@RandomFractals hmm, that's what I get when I run yarn build on my local machine on this branch (master works fine):

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets:
  main.js (288 KiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (288 KiB)
      main.js


WARNING in webpack performance recommendations:
You can limit the size of your bundles by using import() or require.ensure to lazy load some parts of your application.
For more info visit https://webpack.js.org/guides/code-splitting/
Child html-webpack-plugin for "index.html":
     1 asset
    Entrypoint undefined = index.html
       4 modules
$ yarn workspace dvc-integration build
error Unknown workspace "dvc-integration".
info Visit https://yarnpkg.com/en/docs/cli/workspace for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

so, it is seems to be related/fails only on this branch.

We can always review and tighten formatting and linting once we have some basic things working as expected.

we can discuss this later/separately. I see formatting, linting, tests as things that are easy to setup and speedup development significantly. And it's easy to setup them initially and keep code and coverage reasonably clean, but takes a lot of time to setup down the road.

Currently, this vscode extension is very loose in that aspect

also, separate topic. Good points though My suggestion - create a ticket/story with checkboxes for those aspects with some explanation and motivation behind them. We clearly should prioritize this as well.

RandomFractals · 2021-01-04T10:50:50Z

@shcheklein yeah, that doesn't help much. You keep on quoting the same build errors related to webpack and this monorepo.

I did not change webpack scripts & maybe @rogermparent can give it a go and spot a problem.

As I mentioned already the issue is upstream. Extension builds and installs just fine. See screenshots I shared.

I suspect this build issue is related to vscode extension name change and I am not familiar with your build scripts or whatever GH hooks you guys are still adding.

So, in short I am blocked on this, and perhaps another dev more familiar with webpack and your monorepo packaging can look into it.

In the meantime, let's try smaller code change branches merge. Please review & approve #61, #62 & #64.

after vscode extension rename to `dvc`

RandomFractals · 2021-01-04T11:33:56Z

so, I fixed the monorepo build and merged latest from the main branch.

Let's review and merge this today so I can move on to customizing this dvc file tree.

This is what I get locally from building this monorepo now. Should work on your machines too

rogermparent · 2021-01-04T18:29:27Z

I had to jostle around the repo to get it to build, but I did end up getting it working.
It looks like adding @types/node while following Webpack's TS config guide an error I had that looks similar to the runner's build error.

I also renamed the package back to dvc-integration to fix the build, as references throughout the repo were importing it by its old name. If we settle on some new names for the extension and/or the webview, I can ripgrep through and change all references since dvc-integration is a unique string that's easy to search for.

Removing node_modules from the child repos so everything is properly hoisted fixed some lingering issues, and now I can get a build. I'll push these changes to hopefully fix the auto-build, then I'll open up my test VSCode to play around with the repo and make sure it's actually working on my machine.

Remove child package-lock, re-add Webpack TS packages, and revert dvc-integration -> dvc rename

rogermparent · 2021-01-04T18:59:56Z

Looks like my changes fixed the build in GHA too!

Again, if we want to rename dvc-integration I'm totally open to it, reverting it is just the simplest way to fix that aspect of the broken builds while keeping the future possibilty of a rename open (If I changed every import to just dvc, that'd be harder to track down later).

As far as using the addition itself, it works for me and looks like a solid base to build off of when we think of the "business behavior" we want in the file tree (what files to show, what to do when clicking on files, etc.)

This kind of DVC file managemement could, if I understand correctly, also fit well in the SCM view group via the SCM extension API, as DVC is an SCM tool at its core and used alongside Git.
Such a menu could complement the Git plugin it'll be placed next to by only tracking DVC-tracked files that are deliberately ignored in Git. One implementation that comes to mind could be to look for all .dvc files in the repo, and showing them in either a sorted menu or tree that's a subset of the full file tree while showing download status and any info the .dvc file has in the entries.

rogermparent

This PR has built up some housecleaning changes that are worth bringing in.

If I'm correct in thinking that some features that are coded in are broken in practice, we should take action to mitigate that by one of:

Getting file-open clicks working before merge
Removing the non-working code before merge so we can build off of what does work
Merging and making an Issue to fix the command

rogermparent · 2021-01-04T19:05:22Z

extension/src/tree/fileTreeDataProvider.ts

+      treeItem.command = {
+        command: 'vscode.open',
+        title: 'Open File',
+        arguments: [element.uri],
+      };  


Am I supposed to be able to open files from the new tree? This functionality doesn't seem to be working for me.
@RandomFractals can you open files from this tree? The issue may just be on my system.

shcheklein · 2021-01-04T21:07:02Z

@rogermparent thanks for taking a look! Let's please get things you fixed (and other useful fixes) into a separate PR, please. I'm closing this since it has a lot of problems that we are not going to address for now.

RandomFractals added 6 commits January 2, 2021 14:16

Add extension constants

038b98d

Add string normalization utils

f81d486

Add basic file utils

50a996c

for files tree view

Add file info for file stats

88e8d51

Add file tree nodes for file tree view

b50a8d8

Add basic file tree view

189cba1

- fix extension icon and info fields in package.json

update vscode dvc extension dev and other dependencies in package.json

446475f

Updates all but webpack libraries for this extension to use the latest vscode apis, etc.

RandomFractals requested a review from rogermparent January 4, 2021 10:51

RandomFractals added the blocked Issue or pull request blocked due to other dependencies or issues label Jan 4, 2021

RandomFractals and others added 2 commits January 4, 2021 05:16

Fix build-extension monorepo script

516817a

after vscode extension rename to `dvc`

Merge branch 'master' into dvc-tree

f7e46ff

Fix webview build by adding TS types

2378dac

rogermparent added 2 commits January 4, 2021 13:33

Fix builds on my machine

7abc310

Remove child package-lock, re-add Webpack TS packages, and revert dvc-integration -> dvc rename

Merge branch 'master' of github.com:iterative/vscode-dvc into dvc-tree

ceed9b0

rogermparent suggested changes Jan 4, 2021

View reviewed changes

shcheklein closed this Jan 4, 2021

rogermparent mentioned this pull request Jan 5, 2021

package.json housekeeping #65

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dvc tree #53

Dvc tree #53

RandomFractals commented Jan 2, 2021

shcheklein commented Jan 2, 2021

shcheklein commented Jan 2, 2021

RandomFractals commented Jan 2, 2021 •

edited

Loading

shcheklein commented Jan 2, 2021

RandomFractals commented Jan 2, 2021

shcheklein commented Jan 2, 2021

RandomFractals commented Jan 3, 2021 •

edited

Loading

RandomFractals commented Jan 3, 2021 •

edited

Loading

shcheklein commented Jan 3, 2021

RandomFractals commented Jan 4, 2021 •

edited

Loading

RandomFractals commented Jan 4, 2021 •

edited

Loading

rogermparent commented Jan 4, 2021 •

edited

Loading

rogermparent commented Jan 4, 2021 •

edited

Loading

rogermparent left a comment

rogermparent Jan 4, 2021 •

edited

Loading

shcheklein commented Jan 4, 2021

Dvc tree #53

Dvc tree #53

Conversation

RandomFractals commented Jan 2, 2021

shcheklein commented Jan 2, 2021

shcheklein commented Jan 2, 2021

RandomFractals commented Jan 2, 2021 • edited Loading

shcheklein commented Jan 2, 2021

RandomFractals commented Jan 2, 2021

shcheklein commented Jan 2, 2021

RandomFractals commented Jan 3, 2021 • edited Loading

RandomFractals commented Jan 3, 2021 • edited Loading

shcheklein commented Jan 3, 2021

RandomFractals commented Jan 4, 2021 • edited Loading

RandomFractals commented Jan 4, 2021 • edited Loading

rogermparent commented Jan 4, 2021 • edited Loading

rogermparent commented Jan 4, 2021 • edited Loading

rogermparent left a comment

Choose a reason for hiding this comment

rogermparent Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

shcheklein commented Jan 4, 2021

RandomFractals commented Jan 2, 2021 •

edited

Loading

RandomFractals commented Jan 3, 2021 •

edited

Loading

RandomFractals commented Jan 3, 2021 •

edited

Loading

RandomFractals commented Jan 4, 2021 •

edited

Loading

RandomFractals commented Jan 4, 2021 •

edited

Loading

rogermparent commented Jan 4, 2021 •

edited

Loading

rogermparent commented Jan 4, 2021 •

edited

Loading

rogermparent Jan 4, 2021 •

edited

Loading