Introduce salesforcedx-webview-ui package #451

vazexqi · 2018-05-31T22:54:40Z

What does this PR do?

This is the package that will contain all the .html that we will use for each webview. We will use this to create webviews that will be surfaced in VS Code. This will be based on the approach documented at https://code.visualstudio.com/docs/extensions/webview

What issues does this PR fix or reference?

@W-5013822@

codecov · 2018-06-01T00:20:33Z

Codecov Report

Merging #451 into develop will decrease coverage by 0.5%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop     #451      +/-   ##
===========================================
- Coverage    76.38%   75.88%   -0.51%     
===========================================
  Files          138      139       +1     
  Lines         5400     5436      +36     
  Branches       845      851       +6     
===========================================
  Hits          4125     4125              
- Misses        1066     1102      +36     
  Partials       209      209

Impacted Files	Coverage Δ
...dx-vscode-core/src/webviewPanels/manifestEditor.ts	`0% <0%> (ø)`
...scode-core/src/decorators/scratch-org-decorator.ts	`39.13% <0%> (+3.13%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57abc05...73c4449. Read the comment docs.

vazexqi · 2018-06-02T01:03:24Z

packages/salesforcedx-webview-ui/README.md

@@ -0,0 +1,122 @@
+# Introduction


This is the main file that you should read to get the gist of this PR.
The other parts I will illustrate to the team via an internal demo.

@lcampos - Would appreciate your thoughts on this.

We can still merge this in and we can continue the discussion later but it would be appreciated if you could share some thoughts on this based on your work with sutro - I'm not sure if you were using webviews there (or browserview).

Either way, the links in the README.md should be useful.

I did look at using webviews but I was able to make it work by using TextDocumentContentProvider to render my html/js. I didn't need to modify anything outside of the html/js files I was rendering.

Ok, seems WebViews is the "new" way to go based on this https://code.visualstudio.com/updates/v1_21#_webview-api

vazexqi · 2018-06-04T16:29:57Z

packages/salesforcedx-webview-ui/README.md

+
+* [`<webview>` Tag](https://electronjs.org/docs/api/webview-tag)
+* [Interop’s Labyrinth: Sharing Code Between Web & Electron Apps](https://slack.engineering/interops-labyrinth-sharing-code-between-web-electron-apps-f9474d62eccc)
+* [Growing Pains: Migrating Slack’s Desktop App to BrowserView](https://slack.engineering/growing-pains-migrating-slacks-desktop-app-to-browserview-2759690d9c7b)


There are some bugs with webview in Electron. See https://github.com/electron/electron/issues?q=is%3Aopen+is%3Aissue+label%3Acomponent%2Fwebview

However, I searched on the VS Code repo and there were no discussions about considering using BrowserView instead of WebView. So that option doesn't seem to exist for VS Code.

lcampos · 2018-06-05T13:57:03Z

packages/salesforcedx-webview-ui/README.md

+
+# Developer Flow
+
+## Rapid Iteration of the UI


This is currently just showing me a blank page, is it expected ? I do see the manifest editor when I run the extensions tho.

Did you navigate to http://localhost:3000/ManifestEditor

That was it, thanks

lcampos · 2018-06-05T14:14:11Z

packages/salesforcedx-webview-ui/config/webpack.config.dev.js

+            // Also exclude `html` and `json` extensions so they get processed
+            // by webpacks internal loaders.
+            exclude: [/\.(js|jsx|mjs)$/, /\.html$/, /\.json$/],
+            loader: require.resolve('file-loader'),


Did you try something like
{ test: /.(jpe?g|png|gif|svg|woff|eot|ttf)$/i, loader: 'file-loader?name=/media/[name].[ext]' }
? I have it working like that in another project but it might be because I'm on a more recent version of file-loader.

Will try. I wasn't sure if you could pack svg|woff|eot|ttf. That would help.

I might be misunderstanding here but this should be OK because this is the last branch of the oneOf branch. That means we shouldn't need to add more tests. In fact, this test is the not of the other branches.

So, we are already serving the svg, woff, eot, and ttf files. For jpeg, png, gif we actually convert embed them as data URLS to avoid any requests. See

salesforcedx-vscode/packages/salesforcedx-webview-ui/config/webpack.config.dev.js

Line 121 in e84a517

// "url" loader works like "file" loader except that it embeds assets

and https://webpack.js.org/loaders/url-loader/

What I've done is also add woff|eot|ttf so that they get embedded.

I verified that we must be serving them properly because blueprint.js uses some custom font for the icons and we are seeing those icons in the manifest editor.

lcampos · 2018-06-05T14:19:24Z

packages/salesforcedx-webview-ui/package.json

+  "engines": {
+    "vscode": "^1.17.0"
+  },
+  "dependencies": {


Everything webpack, loaders, jest etc should be under devDependencies.

Interesting. I got this from create-react-app. This was the section that they put it in. I can move them into dev-dependencies.

I guess it depends on how we are bundling the extension, are we keeping them as part of node_modules when creating the extension?

Nope, we only bundle the stuff that is optimized in the /build folder. So nothing from node_modules leaks in. Everything has to be bundled in a way that you can just load the .html/.js/etc. All the JS is bundled as UMD, not That's why I think it's OK and that is why create-react-app did it that way. I will investigate more but I don't think I change this from how they were doing it.

I'm looking at https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/package.json and https://github.com/wmonk/create-react-app-typescript/blob/master/packages/react-scripts/package.json

It seems that react, react-dom, and typescript are in the devDependencies. Let me move those back in there to follow that (not sure how it got moved into dependencies).

I will keep webpack and rest in dependencies for now since that seems to be how the template it.

More info at facebook/create-react-app#1764

lcampos · 2018-06-05T14:20:12Z

packages/salesforcedx-webview-ui/package.json

+    "tslint-react": "3.2.0",
+    "uglifyjs-webpack-plugin": "1.1.8",
+    "url-loader": "0.6.2",
+    "webpack": "3.8.1",


lcampos · 2018-06-05T14:35:00Z

packages/salesforcedx-webview-ui/src/entries/ManifestEditor/index.tsx

@@ -0,0 +1,6 @@
+import * as React from 'react';
+import * as ReactDOM from 'react-dom';


We should change this to import { render } from 'react-dom'; so it will only import the render method and nothing else.

Doesn't the tree shaking and dead-code elimination from webpack solve the issue of bringing too much? Or am I overestimating what it can do?

Tree shaking will remove that, but I didn't see anything for that on the webpack config files. Is babel compiling to ES6? If it is then webpack will automatically do tree shaking.

salesforcedx-vscode/packages/salesforcedx-webview-ui/config/webpack.config.prod.js

Line 284 in e84a517

ecma: 5,

Looks like we are still compressing/outputting to ecma5. Let me get it into ecma 6. We should use es6 to be consistent with the rest of the output from vs code.

I see now that tree shaking is only enabled for es6 or es2015. https://webpack.js.org/guides/tree-shaking/

This is the package that will contain all the .html that we will use for each webview. @W-5013822@

Nothing fancy but it showcases the behavior and ability.

Since this is tied to the salesforcedx-vscode-core extension that only loads when a sfdx-project.json is around.

vazexqi · 2018-06-05T20:33:14Z

@lcampos - The current version of create-react-app uses webpack 3.8.1 and not webpack 4 yet. We can decide to update this later if needed. I can't find an official upgrade doc like I could for webpack 2 -> 3 (https://webpack.js.org/migrate/3/)

lcampos · 2018-06-06T14:17:57Z

@vazexqi there's not migration guide yet but there's some progress being made and tracked here (webpack/webpack.js.org#1706). Right now, unless we see build/bundle perf issues I'd say we can wait to move to v4.

vazexqi mentioned this pull request Jun 1, 2018

Bump vscode dependency to 1.23 #452

Merged

vazexqi commented Jun 2, 2018

View reviewed changes

vazexqi force-pushed the nick/add-salesforcedx-webview-ui-package branch from 446a2b0 to c9689fe Compare June 2, 2018 01:11

vazexqi requested a review from lcampos June 4, 2018 15:52

vazexqi commented Jun 4, 2018

View reviewed changes

lcampos reviewed Jun 5, 2018

View reviewed changes

lcampos approved these changes Jun 5, 2018

View reviewed changes

Nick Chen added 17 commits June 5, 2018 13:25

Create a webview-ui package

d8a93a8

This is the package that will contain all the .html that we will use for each webview. @W-5013822@

Add commands to help build and copy things over

221e79d

Bump VS Core Requirement to 1.23 to support WebView

640eb2a

Clarify the new steps for developing

d311f1b

Prevent double copying during build of salesforcedx-webviews-ui

409d0df

Remove unnecessary sample from salesforcedx-webview-ui

c27e91f

Initial working version

06fc480

Nothing fancy but it showcases the behavior and ability.

Fix tslint errors

8e6e5b6

Update to Task 2.0 format

96db4ed

Compile needs to be serial due to dependencies

dadf1b3

Add README

3222faa

Tie command to sfdx:project_opened

b5af5f1

Since this is tied to the salesforcedx-vscode-core extension that only loads when a sfdx-project.json is around.

Clarify README

72a85f3

Straighten diagram

07024cc

Restore react and react-dom to devDependencies

659224e

Output and compress to es6 to enable tree-shaking

863ad30

Add fonts (eot, ttf, woff) as list of items that can embedded

23bf1c2

vazexqi force-pushed the nick/add-salesforcedx-webview-ui-package branch from db9c92e to 23bf1c2 Compare June 5, 2018 20:25

Merge branch 'develop' into nick/add-salesforcedx-webview-ui-package

73c4449

vazexqi merged commit 8da30f8 into develop Jun 5, 2018

vazexqi deleted the nick/add-salesforcedx-webview-ui-package branch June 5, 2018 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce salesforcedx-webview-ui package #451

Introduce salesforcedx-webview-ui package #451

vazexqi commented May 31, 2018

codecov bot commented Jun 1, 2018 •

edited

Loading

vazexqi Jun 2, 2018

vazexqi Jun 4, 2018

lcampos Jun 5, 2018

lcampos Jun 5, 2018

vazexqi Jun 4, 2018

lcampos Jun 5, 2018

vazexqi Jun 5, 2018

lcampos Jun 5, 2018

lcampos Jun 5, 2018 •

edited

Loading

vazexqi Jun 5, 2018

vazexqi Jun 5, 2018

lcampos Jun 5, 2018

vazexqi Jun 5, 2018

lcampos Jun 5, 2018

vazexqi Jun 5, 2018

vazexqi Jun 5, 2018

vazexqi Jun 5, 2018

lcampos Jun 5, 2018

lcampos Jun 5, 2018

vazexqi Jun 5, 2018

lcampos Jun 5, 2018

vazexqi Jun 5, 2018

vazexqi commented Jun 5, 2018

lcampos commented Jun 6, 2018

		@@ -0,0 +1,6 @@
		import * as React from 'react';
		import * as ReactDOM from 'react-dom';

Introduce salesforcedx-webview-ui package #451

Introduce salesforcedx-webview-ui package #451

Conversation

vazexqi commented May 31, 2018

What does this PR do?

What issues does this PR fix or reference?

codecov bot commented Jun 1, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcampos Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vazexqi commented Jun 5, 2018

lcampos commented Jun 6, 2018

codecov bot commented Jun 1, 2018 •

edited

Loading

lcampos Jun 5, 2018 •

edited

Loading