Use wireit to run scripts #241

rictic · 2022-04-06T18:49:34Z

This lets scripts specify their dependencies, avoid re-doing work unnecessarily, and has a robust watch mode for free!

aomarks · 2022-04-06T19:07:00Z

package.json

-		"copylink": "node scripts.js copylink",
-		"copylink:watch": "nodemon --exec \"npm run copylink\""
+		"copylink": "wireit",
+		"copylink:watch": "nodemon --exec \"npm run copylink\"",


Could this just now be replaced with npm run copylink watch? Then could drop the nodemon dependency too.

yes, and probably many of the other watch scripts can be removed too, just need to evaluate them more

ava's watch mode is potentially useful though, the ava tests take a little while

aomarks · 2022-04-06T19:15:18Z

packages/vscode-lit-plugin/package.json

+				"../lit-analyzer:build",
+				"../../:copylink"
+			],
+			"command": "tsc --build",


You'll want "clean": false here, otherwise the output will get deleted on every run. If the .tsbuildinfo is in output, that would work, but there would be no advantage to --build. If it's not in output, I think it will actually be broken, because tsc thinks previous up-to-date output exists that actually doesn't.

So you want "clean": false and to include the .tsbuildinfo file in output. When google/wireit#70 lands, you can then switch to "clean": "on-delete", which will give you the best result because you'll get incremental build on changes + new files, and a clean build on deleted files (which is better than the default behavior of tsc --build, which just leaves compiled output for deleted source files lying around).

packages/lit-analyzer/package.json

aomarks · 2022-04-06T19:16:26Z

packages/ts-lit-plugin/package.json

+			"dependencies": [
+				"../lit-analyzer:build"
+			],
+			"command": "tsc --build",


See above comment. Add "clean": false and add .tsbuildinfo to output.

aomarks · 2022-04-06T19:18:14Z

package.json

+				"./packages/ts-lit-plugin:build"
+			],
+			"files": [
+				"packages/lit-analyzer",


This doesn't work at the moment -- for input files, including a directory does not implicitly include the files inside it. You'll need to write e.g. packages/lit-analyze/** for now (planning a fix for this soon).

packages/lit-analyzer/package.json

Also, we need to explicitly specify the tsbuildinfo files that each tsconfig should use, since the compiler starts guessing weird file names for them if we don't (e.g. ../tsconfig.tsbuildinfo)

aomarks · 2022-04-11T22:08:40Z

package.json

+			],
+			"command": "node scripts.js copylink"
+		},
+		"eslint": {


This is fine, but just FYI these could also be regular old npm scripts, since they dont have any dependencies, files, output. Wireit scripts can depend on non-Wireit scripts.

aomarks · 2022-04-11T22:09:30Z

packages/lit-analyzer/package.json

+				"index.js",
+				"index.d.ts",
+				"index.d.ts.map",
+				"./.tsbuildinfo"


Suggested change

"./.tsbuildinfo"

".tsbuildinfo"

aomarks · 2022-04-11T22:09:58Z

packages/lit-analyzer/package.json

+				"build"
+			],
+			"files": [
+				"package.json"


Assuming this is not a build artifact:

Suggested change

"package.json"

"package.json",

"scripts/check-version.js"

aomarks · 2022-04-11T22:11:58Z

packages/vscode-lit-plugin/package.json

+				"schemas/**/*"
+			],
+			"output": [],
+			"command": "node ./out/test/scripts/test-runner.js"


Looks like this is a build artifact, so it doesn't need to be in files. (Just confirming?)

Same for the other scripts below.

Yes, that's right

aomarks · 2022-04-11T22:14:03Z

packages/vscode-lit-plugin/package.json

+				"./out/packaged.vsix"
+			],
+			"output": [
+				"../../packaged-extension/extension/**/*"


Currently it's forbidden to have output outside of the package directory. It will crash when you try to do a clean build (just as an intentional check, because it seemed potentially indicative of an error) or cache (because we'll need a different directory layout to support this).

If you don't clean or cache here, it's OK though.

Gotcha, dropped the output array then

aomarks · 2022-04-11T22:15:17Z

packages/vscode-lit-plugin/src/test/scripts/test-runner.ts

@@ -23,6 +23,10 @@ async function main() {
 		const fixturesDir = path.join(__dirname, "..", "..", "..", "src", "test", "fixtures");
 		// Download VS Code, unzip it and run the integration test
 		await runTests({ extensionDevelopmentPath, extensionTestsPath, launchArgs: [fixturesDir] });
+
+		setTimeout(function () {


Is this a leftover?

Seems like it could cause a hanging test to exit with a success code and report a false success?

If flow control reaches this point then the tests have finished successfully. Added more comments.

43081j · 2022-04-12T11:52:25Z

TIL wireit is a thing. is there a readme anywhere for it? would be good to understand how it works more

also nice to see @aomarks finally got pulled into this 😂

aomarks · 2022-04-12T16:06:15Z

TIL wireit is a thing. is there a readme anywhere for it? would be good to understand how it works more

https://github.com/google/wireit It's still a private repo (probably just for a few more days). Added you to it, though, so you can check it out!

rictic added 3 commits April 5, 2022 17:43

Bump version of vsce so we can publish a pre-release.

90e14b6

Use wireit to run scripts

578b68a

This lets scripts specify their dependencies, avoid re-doing work unnecessarily, and has a robust watch mode for free!

Drop older node versions.

313bc97

aomarks suggested changes Apr 6, 2022

View reviewed changes

rictic added 3 commits April 6, 2022 12:47

Update typescript to fix build.

f0f3d3c

Dont clean on build, emit version info.

b98e3a3

Hack hack hack

fa96928

rictic marked this pull request as draft April 6, 2022 22:44

rictic added 2 commits April 6, 2022 15:53

Hack hack hack

b1f75f7

hack hack hack hack hack

6ac4502

aomarks mentioned this pull request Apr 7, 2022

Update PATH depending on current package google/wireit#85

Merged

rictic added 10 commits April 7, 2022 08:27

hack hack hack hack hack hack hack

649169c

Remove out of date comment

fdbe5a9

Uncomment out a line

e65c640

Sequentialize vscode tests.

1422d89

hack hack hack hack hack

6431c03

hack hack

8705aea

hack hack?

a6f9ba6

hack hack

8f352e8

hack hack

d78c0db

hack hack hack cry cry cry

1e4a9dc

rictic marked this pull request as ready for review April 11, 2022 16:59

aomarks reviewed Apr 11, 2022

View reviewed changes

packages/lit-analyzer/package.json Outdated Show resolved Hide resolved

packages/lit-analyzer/package.json Outdated Show resolved Hide resolved

Use recommended wireit tsc settings

e6672ec

Also, we need to explicitly specify the tsbuildinfo files that each tsconfig should use, since the compiler starts guessing weird file names for them if we don't (e.g. ../tsconfig.tsbuildinfo)

aomarks reviewed Apr 11, 2022

View reviewed changes

aomarks suggested changes Apr 11, 2022

View reviewed changes

rictic added 2 commits April 12, 2022 15:52

Address review comments

12cdd1d

Clarify further

89f76e6

rictic self-assigned this Apr 13, 2022

aomarks approved these changes Apr 13, 2022

View reviewed changes

rictic merged commit 8945611 into master Apr 13, 2022

rictic deleted the wireit branch April 13, 2022 18:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use wireit to run scripts #241

Use wireit to run scripts #241

rictic commented Apr 6, 2022

aomarks Apr 6, 2022

rictic Apr 11, 2022

aomarks Apr 6, 2022

rictic Apr 11, 2022

aomarks Apr 6, 2022

rictic Apr 7, 2022

aomarks Apr 6, 2022 •

edited

Loading

aomarks Apr 11, 2022

rictic Apr 12, 2022

aomarks Apr 11, 2022

rictic Apr 12, 2022

aomarks Apr 11, 2022

rictic Apr 12, 2022

aomarks Apr 11, 2022

rictic Apr 12, 2022

aomarks Apr 11, 2022

rictic Apr 12, 2022

aomarks Apr 11, 2022

rictic Apr 12, 2022

43081j commented Apr 12, 2022

aomarks commented Apr 12, 2022 •

edited

Loading

Use wireit to run scripts #241

Use wireit to run scripts #241

Conversation

rictic commented Apr 6, 2022

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

aomarks Apr 6, 2022 • 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

43081j commented Apr 12, 2022

aomarks commented Apr 12, 2022 • edited Loading

aomarks Apr 6, 2022 •

edited

Loading

aomarks commented Apr 12, 2022 •

edited

Loading