Add a TypeScript pass, and revive package.json pass #63

aomarks · 2018-01-24T06:44:19Z

This pass updates an element repo to include typings generated by https://github.com/Polymer/gen-typescript-declarations/ and configures an update-types NPM script that can be run to re-generate them.

It also revives the existing package.json pass to be much simpler (just the most minimal possible package.json), and to update the gitignore file.

Also:

Replace custom node-git typings with the Definitely Typed version.
Add custom typings for BowerConfig and NpmConfig.
Add a package-lock.json.
Bump engines and types to Node 8 so we can use util.promisify.

This pass updates an element repo to include typings generated by https://github.com/Polymer/gen-typescript-declarations/ and configures an "update-types" NPM script that can be run to re-generate them. Also: - Replace custom node-git typings with the Definitely Typed version. - Add a package-lock.json.

TimvdLippe

Maybe also update travis to fail if the types were not updated?

aomarks · 2018-01-24T07:14:39Z

Maybe also update travis to fail if the types were not updated?

Good idea. Same idea as your Polymer/polymer#5003. Assuming we are happy with the slight extra time this will add to every Travis run. Looks like we already have a Travis pass, so hopefully we can just add it there.

justinfagnani · 2018-01-24T15:32:49Z

cleanup-passes/typescript.ts

+  try {
+    packageJson = await fse.readJson(packageJsonPath);
+  } catch {
+    throw new Error(


why not make a minimal one here? I think you can get away with just having scripts and devDeps.

Ok -- I rewrote the existing package-json pass to create a super minimal one, and added it to this PR. It looks like the previous code there was pretty out of date and is now superseded by modulizer.

justinfagnani · 2018-01-24T15:37:57Z

cleanup-passes/typescript.ts

+        `Missing or invalid package.json for element: ${element.ghRepo.name}`);
+  }
+
+  if (packageJson.devDependencies === undefined) {


You can be a little more declarative if you want with object spread:

try { packageJson = { scripts: {}, devDependencies: {}, ...await fse.readJson(packageJsonPath); }; } ...

Nice, although since I'm not used to that pattern I think it would have taken me a sec to figure out what was going on.

I've added a new NpmConfig type (shared with the new package-json pass, see above) which declares these fields as undefined. TypeScript doesn't infer that they can't be undefined with the suggested snippet, so another reason I think I'll stick with mine.

justinfagnani · 2018-01-24T15:41:12Z

cleanup-passes/typescript.ts

+      // If all we did was update the package lock, don't bother with the
+      // commit. But do include it if we changed something else.
+      commitFiles.push(filepath);
+    }


do we expect other file paths? If not, should we log unexpected ones?

Added a console.log to point these out.

justinfagnani · 2018-01-24T15:47:37Z

cleanup-passes/typescript.ts

+  // We also have to bower install because the generator needs to see the
+  // dependencies.
+  await execFilePromise('bower', ['install'], execOpts);
+  await execFilePromise('npm', ['run', npmScriptName], execOpts);


my add a comment to highlight that this actually runs the generator

It looks like previously this was an early attempt at generating package.jsons for actually publishing. Now that's done by modulizer, so all we need here is to create a super minimal one. It also updates .gitignore to ignore node_modules. If any package.json exists, this pass now does absolutely nothing.

aomarks · 2018-01-25T07:18:51Z

PTAL!

aomarks · 2018-01-25T08:04:55Z

I'm waiting on Polymer/gen-typescript-declarations#71 and Polymer/gen-typescript-declarations#72 to be published before I pull the trigger on running this pass.

aomarks · 2018-02-02T07:09:18Z

I've run this on more than half the element repos. Tracking at https://github.com/Polymer/gen-typescript-declarations/issues/79.

@justinfagnani I think this can be committed now, PTAL?

aomarks · 2018-02-06T23:28:39Z

Maybe also update travis to fail if the types were not updated?

This is done.

TimvdLippe · 2018-02-06T23:32:15Z

cleanup-passes/typescript.ts

+        // Update the types.
+        'npm run update-types && ' +
+        // If there were any changes, this git command will return non-zero.
+        'git diff --exit-code || ' +


🎉 🎉 🎉 🎉

That error echo is really nice. Maybe update the script in polymer/polymer as well? 😄

That error echo is really nice. Maybe update the script in polymer/polymer as well? 😄

Done in Polymer/polymer#5099

justinfagnani · 2018-02-06T23:32:23Z

cleanup-passes/package-json.ts

+    // npm warns if any of these fields aren't set.
+    description: bowerConfig.description,
+    repository: bowerConfig.repository,
+    license: bowerConfig.license,


The bower config license field is not a standard SPDX license identifier that package.json should have. I'd just hard code this as "BSD-3-Clause"

justinfagnani · 2018-02-06T23:32:57Z

cleanup-passes/package-json.ts

-'use strict';
-
-import * as fs from 'fs';
+import * as fse from 'fs-extra';


I think we can just import this as fs

I much prefer giving it a distinct name, since actually all the APIs I use are not part of the standard fs, and I've seen fse in some other repos.

aomarks requested a review from justinfagnani January 24, 2018 07:04

TimvdLippe reviewed Jan 24, 2018

View reviewed changes

justinfagnani approved these changes Jan 24, 2018

View reviewed changes

aomarks added 2 commits January 24, 2018 23:16

PR suggestions.

298431c

aomarks changed the title ~~Add a TypeScript pass.~~ Add a TypeScript pass, and revive package.json pass Jan 25, 2018

Remove extra slashes.

26c877b

aomarks added 4 commits January 25, 2018 14:36

Use 1.0.0 of generator.

4186f1f

Don't add extra newlines to gitignore.

e58988a

Updating types depends on bower. And some other tweaks.

141abcf

Bump generator version.

a297a04

aomarks mentioned this pull request Feb 6, 2018

Add TypeScript declarations to iron-overlay-behavior. PolymerElements/iron-overlay-behavior#261

Merged

Add Travis script, and bump generator version.

65b941e

TimvdLippe reviewed Feb 6, 2018

View reviewed changes

justinfagnani approved these changes Feb 6, 2018

View reviewed changes

aomarks added 3 commits February 6, 2018 15:34

Commit travis changes.

684fb20

BSD-3-Clause

2f4739b

Fix false

598f76a

aomarks merged commit db10790 into master Feb 7, 2018

aomarks deleted the typescript branch February 7, 2018 22:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a TypeScript pass, and revive package.json pass #63

Add a TypeScript pass, and revive package.json pass #63

aomarks commented Jan 24, 2018 •

edited

Loading

TimvdLippe left a comment

aomarks commented Jan 24, 2018

justinfagnani Jan 24, 2018

aomarks Jan 25, 2018

justinfagnani Jan 24, 2018

aomarks Jan 25, 2018

justinfagnani Jan 24, 2018

aomarks Jan 25, 2018

justinfagnani Jan 24, 2018

aomarks Jan 25, 2018

aomarks commented Jan 25, 2018

aomarks commented Jan 25, 2018

aomarks commented Feb 2, 2018

aomarks commented Feb 6, 2018

TimvdLippe Feb 6, 2018

aomarks Feb 7, 2018

justinfagnani Feb 6, 2018

aomarks Feb 6, 2018

justinfagnani Feb 6, 2018

aomarks Feb 6, 2018

Add a TypeScript pass, and revive package.json pass #63

Add a TypeScript pass, and revive package.json pass #63

Conversation

aomarks commented Jan 24, 2018 • edited Loading

TimvdLippe left a comment

Choose a reason for hiding this comment

aomarks commented Jan 24, 2018

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

aomarks commented Jan 25, 2018

aomarks commented Jan 25, 2018

aomarks commented Feb 2, 2018

aomarks commented Feb 6, 2018

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 commented Jan 24, 2018 •

edited

Loading