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 VSC UI Toolkit #1721

Merged
merged 4 commits into from
Jul 12, 2022
Merged

Add VSC UI Toolkit #1721

merged 4 commits into from
Jul 12, 2022

Conversation

AngelMunoz
Copy link
Contributor

@AngelMunoz AngelMunoz commented Jun 12, 2022

This PR attempts to ease a little bit working with webviews and to also enable the usage of the vscode ui toolkit.

I didn't go as far as thinking what else can we use webviews for but I at least switched the table for the watcher for the datagrid included in the toolkit.

There are a few observations I have on this regard

  • I had to include the toolkit js file on the release directory

    This might be able to be solved with a few tweaks on the webpack config but I couldn't figure it out

  • I tried using Feliz.ViewEngine but I decided to stick with strings

    The vscode toolkit would require the proper bindings for Feliz.ViewEngine and that might be quite a lot of work, thankfully Alfonso has already make it easier to work with html/css/js strings within F# files thanks to his extension so I felt it was actually easier to work with inline strings

  • Webviews could be made with an project outside src

    After my comment on Consider managing FSI sessions in FSAC directly #1720 I found out that yeah we could use external websites for the webviews but they have to live in iframes which make things quite complicated, what what we could do is to have a separate project (Fable.Lit or Feliz based one) that outputs the final bundle to the release directory, later on we would build the index by hand (as we're doing it now) but including the scripts within the release directory. That complicates infrastructure though so it might be just too much for the project to handle at least right now.

In any case Feel free to close this PR if this is not needed it was more to explore approaches and see what worked and what not, I thought it could be useful so I ventured to open this.

@baronfel
Copy link
Contributor

Haven't looked at this in detail, but I am looking at moving to esbuild in #1684/#1713 so maybe that would be easier?

Copy link
Contributor

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

Thanks! I like the contribution, its a nice styling and UX enhancement for sure. I'd like to see if the packaging issue can be fixed - ideally we'd be able to NPM reference the toolkit and let the build take care of bundling the output.

.gitignore Outdated

# include vscode ui toolkit minified release file
!release/toolkit.min.js
Copy link
Contributor

Choose a reason for hiding this comment

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

super weird - can you add not a reference to the toolkit and let webpack pull it in (I'm just skimming their getting started docs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried following the getting started from the toolkit but their getting started assumes the directory where you launch your extension is the root of the project (hence why it had node_modules available), in our case the release directory is the root so we don't have access to node_modules, we'd likely need to do some webpack magic to pull this script at dev/build which was the part I couldn't figure out

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. I'd also be ok longer term with removing the release split. It's just confusing IMO.

@AngelMunoz
Copy link
Contributor Author

give me a chance to look over this week/next weekend to remove webpack from the toolchain
technically we don't need it anymore the electron version handles all of the JS Fable emits, so we would likely need something like rollup which helps us with minification and tree-shaking, watch mode and other helpful stuff

I don't really think we need vite/esbuild because we don't really run in a browser context we're basically just doing node stuff under the hood

This was referenced Jun 12, 2022
@Krzysztof-Cieslak
Copy link
Member

I like this, let's merge it.

@Krzysztof-Cieslak Krzysztof-Cieslak merged commit c45b8fc into ionide:main Jul 12, 2022
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

Successfully merging this pull request may close these issues.

3 participants