feat(cli): add vscode config files #1336

shimks · 2018-05-16T21:27:56Z

todo: docs
closes Enable tsc watch in projects scaffolded by lb4 CLI tool #1259

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

raymondfeng · 2018-05-16T21:45:29Z

examples/hello-world/.vscode/launch.json

+        "dist*/test/**/*.js",
+        "-t",
+        "0"
+      ]


I don't think we have tests for hello-world.

IMO, this should be there in case users want to build on top of hello-world

raymondfeng · 2018-05-16T21:46:28Z

examples/log-extension/.vscode/launch.json

+      "cwd": "${workspaceRoot}",
+      "args": [
+        "--opts",
+        "${workspaceRoot}/test/mocha.opts",


I don't think we have test/mocha.opts for this module.

bajtos · 2018-05-17T07:09:47Z

examples/hello-world/.vscode/launch.json

+      "args": [
+        "--opts",
+        "${workspaceRoot}/test/mocha.opts",
+        "dist*/test/**/*.js",


This will not work. npm run build produces both dist8 and dist10 directories. Your wildcard will match both Node.js 8.x and 10.x dist files, therefore each test will be run twice, and if you run on Node.js 8.x, the 10.x tests will fail on unsupported syntax/APIs.

bajtos · 2018-05-17T07:10:42Z

examples/log-extension/.vscode/launch.json

+      "args": [
+        "--opts",
+        "${workspaceRoot}/test/mocha.opts",
+        "dist*/test/**/*.js",


Ditto, dist* will not work well.

bajtos · 2018-05-17T07:11:46Z

examples/hello-world/package.json

+    "pretest": "npm run clean && npm run build:current",
+    "test": "lb-mocha --allow-console-logs \"DIST/test\"",
+    "posttest": "npm run lint",
+    "test:dev": "lb-dist mocha DIST/test/**/*.js && npm run lint",


I think we should be using lb-mocha instead of lb-dist mocha in all test:dev scripts, thoughts?

shimks · 2018-05-18T19:21:21Z

packages/cli/generators/project/templates/.vscode/launch.json.ejs

+      "name": "Run Mocha tests",
+<% if (project.loopbackBuild) { -%>
+      "autoAttachChildProcesses": true,
+      "program": "${workspaceRoot}/node_modules/.bin/lb-mocha",


There's a problem here where if we were using this config to debug and we end up hitting the restart button, we get timed out with an ECONNREFUSED error. The problem seems to be that the processes and its child processes don't exit out properly. I'd like to get some help with this if possible.

IIRC, you need to invoke _mocha directly if you want to debug the tests.

lb-mocha creates a child process that creates another node process mocha which creates the third child process `_mocha.

Maybe we should add a new option to lb-mocha to tell our wrapper to start mocha in a debug mode? I am not sure if that would work with VS Code inspector though.

If it's easier, then I am ok with leaving out launch.json configuration from this initial pull request.

We can already start mocha in debug mode by passing in --inspect as an argument, but that doesn't seem to fix the problem of the timeout issue when restarting. This might be fixable if lb-mocha calls _mocha instead of mocha, but I think I'd rather address this in a separate issue.

Fair enough 👍 Looks like you already understand this matter better than I do 😄

virkt25

Great start. For the examples/* projects there is a lot of repetition on prettierrc and prettierignore files. Can we include these in @loopback/build and then pass in the path to prettier via flags:

--config ./node_modules/@loopback/build/config/prettier/.prettierrc and
--ignore-path ./node_modules/@loopback/build/config/prettier/.prettierignore in package.json

this way if we change config it only needs to happen in one place.

virkt25 · 2018-05-23T19:18:23Z

examples/hello-world/.vscode/tasks.json

+      "label": "Build, Test and Lint",
+      "type": "shell",
+      "command": "npm",
+      "args": ["--silent", "run", "test:dev"],


Shouldn't this be test?

The 'advocated' development pattern according to DEVELOPING.md was to have build:watch running and then running the tests without building (since build:watch takes care of that). test:dev is just npm t without the build step

Ah ok. Makes sense. 👍

virkt25 · 2018-05-23T19:19:04Z

examples/hello-world/.vscode/tasks.json

+      "label": "Watch and Compile Project",
+      "type": "shell",
+      "command": "npm",
+      "args": ["--silent", "run", "build:watch"],


There is no matching build:watch in package.json.

good catch!

bajtos

LGTM as far as I am concerned, please work with @virkt25 to iron out the details he pointed out.

virkt25 · 2018-05-25T04:33:58Z

examples/hello-world/package.json

+    "pretest": "npm run clean && npm run build:current",
+    "test": "lb-mocha --allow-console-logs \"DIST/test\"",
+    "posttest": "npm run lint",
+    "test:dev": "lb-mocha DIST/test/**/*.js && npm run lint",


Change npm run lint to npm run posttest -- this will help ensure consistency with the test command.

I think doing npm run test -- --skip-scripts && npm run posttest might be an overkill possible but will ensure the greatest consistency between the scripts.

~~I like the npm run posttest idea, but I'm a bit tripped up on the npm run test -- --skip-scripts. Is the intention of that to skip the build process?~~

Nvm, figured out what you meant. Unfortunately, --skip-scripts also disables npm test because it's also considered a script. I know what consistency you want though, so I'm going to find out if there's a way to disable hooks from tests any other way. If there isn't, 🤷‍♂️

shimks · 2018-05-25T17:52:52Z

@virkt25 The use case for prettier config files IMO is for the users when they lb4 example the apps with the config files in them. With .prettierrc and .prettierignore provided for them, they should be able to easily configure the linter however they want with the added bonus of being able to be used by the prettier extension from VSCode (I think this is the big one).

shimks requested review from b-admike, bajtos, raymondfeng and virkt25 as code owners May 16, 2018 21:27

raymondfeng reviewed May 16, 2018

View reviewed changes

bajtos reviewed May 17, 2018

View reviewed changes

bajtos suggested changes May 17, 2018

View reviewed changes

shimks force-pushed the cli/vscode branch 2 times, most recently from 0b1f63c to ce132b6 Compare May 18, 2018 19:17

shimks commented May 18, 2018

View reviewed changes

shimks mentioned this pull request May 22, 2018

Using lb-mocha with VSCode's debug feature results in an error #1348

Closed

shimks force-pushed the cli/vscode branch from ce132b6 to 89da29e Compare May 22, 2018 15:48

virkt25 reviewed May 23, 2018

View reviewed changes

bajtos approved these changes May 24, 2018

View reviewed changes

virkt25 reviewed May 25, 2018

View reviewed changes

shimks force-pushed the cli/vscode branch from f74bff8 to f3f0a19 Compare May 25, 2018 18:04

virkt25 approved these changes May 31, 2018

View reviewed changes

feat(cli): add vscode config files

27d0a78

shimks force-pushed the cli/vscode branch from f3f0a19 to 27d0a78 Compare June 1, 2018 14:32

shimks merged commit 3738b9c into master Jun 1, 2018

shimks deleted the cli/vscode branch June 1, 2018 14:45

bajtos mentioned this pull request Jul 30, 2018

Setup tsc --watch workflow in examples and CLI project templates #922

Closed

3 tasks

feat(cli): add vscode config files #1336

feat(cli): add vscode config files #1336

Uh oh!

Conversation

shimks commented May 16, 2018

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virkt25 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bajtos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shimks May 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shimks commented May 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

virkt25 left a comment •

edited

Loading

shimks May 25, 2018 •

edited

Loading