Create TS types in `mage build` by jfsiii · Pull Request #196 · elastic/package-registry

jfsiii · 2020-01-15T19:31:14Z

Uses https://github.com/OneOfOne/struct2ts to generate types.d.ts file from util.Package struct.

Added the minimum features required to support consuming the TS types in EPM. We can build on this with a proper build/release/publish process, new/improved types, etc.

Will add more notes inline.

Created a PR in EPM elastic/kibana#54955 which consumes these types

closes #195

Use github.com/OneOfOne/struct2ts to generate `types.d.ts` file from `util.Package` struct.

jfsiii · 2020-01-15T19:38:46Z

types.js

@@ -0,0 +1,2 @@
+"use strict";


Added the bare minimum required to get EPM to be able to import '@elastic/package-registry'.

This is the result when did things The Right Way locally and avoids bringing in TypeScript, running tsc --declaration ..., etc each time.

We can move to that later. For now, new golang structs will still be written to types.d.ts and available to consumers just by running mage build

jfsiii · 2020-01-15T19:42:29Z

package.json

+  "description": "Elastic Package Registry (EPR)",
+  "repository": "git@github.com:elastic/package-registry.git",
+  "author": " Nicolas Ruflin <spam@ruflin.com>",
+  "license": "Elastic-License",


npm/yarn print a warning because this is not available in the SPDX License List. Perhaps we could create a PR to be included. Mongo's SSPL is in the list

Or we make the definition itself Apache licensed?

Maybe when/if we split types out to their own project/repo. Or we should change the name/files. This is called @elastic/package-registry and actually brings in all the other repo files so the Elastic license is appropriate for what it's currently doing.

jfsiii · 2020-01-15T19:43:23Z

package.json

@@ -0,0 +1,10 @@
+{
+  "name": "@elastic/package-registry",
+  "version": "0.2.0",


Ideally this would be linked to the version in BuildRootFile so they're always the same

We probably will have to generate it. Or does it allow linking here?

What I was imagining was BuildRootFile would read package.json to get the version.

package.json would be managed with npm version command or something similar.

The root file would remain synced and the build script doesn't have to take on knowledge of building another artifact.

Version would gain a source of truth that can be read by other tools instead of being inside a script.

+1 on having it in a file. I wonder if package.json is the right place for it considering it is a Golang tool, but in the end it is just JSON, so ... Will figure it out as soon as we automate it.

jfsiii · 2020-01-15T19:44:56Z

package.json

+  "repository": "git@github.com:elastic/package-registry.git",
+  "author": " Nicolas Ruflin <spam@ruflin.com>",
+  "license": "Elastic-License",
+  "types": "./types.d.ts",


This is the file written by BuildTypesFile. It's currently possible to update the magefile and break the package but I left that linking for a later phase.

ruflin

Left a few question. Code in general LGTM.

I think at one stage we will move the "definition" of a package outside the registry itself and these files will go with it. But for now good to have it all in one place.

ruflin · 2020-01-16T07:34:24Z

package.json

@@ -0,0 +1,10 @@
+{


Do we need this on the top level? I would prefer to not have generated files in the root.

Yes, package.json needs to be at the root.

I don't think this is a generated file. Or at least it doesn't change as the result of a mage command, for example.

having package.json at the root SGTM

ruflin · 2020-01-16T07:35:02Z

package.json

+  "description": "Elastic Package Registry (EPR)",
+  "repository": "git@github.com:elastic/package-registry.git",
+  "author": " Nicolas Ruflin <spam@ruflin.com>",
+  "license": "Elastic-License",


Or we make the definition itself Apache licensed?

ruflin · 2020-01-16T07:35:31Z

types.d.ts

@@ -0,0 +1,59 @@
+// this file was automatically generated, DO NOT EDIT


I don't like the location of this file. Could we put it with package under npm or js or a similar directory?

I initially put it in docs because I forgot the second part of this comment. I just moved it to a new types directory.

ruflin · 2020-01-16T07:35:58Z

types.js

@@ -0,0 +1,2 @@
+"use strict";


Also going into a sub directory?

same as #196 (comment)

ruflin

Change LGTM. Could you add a changelog entry?

ruflin · 2020-01-20T08:16:41Z

package.json

+  "description": "Elastic Package Registry (EPR)",
+  "repository": "git@github.com:elastic/package-registry.git",
+  "author": " Nicolas Ruflin <spam@ruflin.com>",
+  "license": "Elastic-License",


ruflin · 2020-01-20T08:17:02Z

package.json

@@ -0,0 +1,10 @@
+{
+  "name": "@elastic/package-registry",
+  "version": "0.2.0",


We probably will have to generate it. Or does it allow linking here?

ruflin · 2020-01-20T08:17:16Z

package.json

@@ -0,0 +1,10 @@
+{


having package.json at the root SGTM

ruflin · 2020-01-20T08:18:15Z

package.json

+  "author": " Nicolas Ruflin <spam@ruflin.com>",
+  "license": "Elastic-License",
+  "types": "./types/types.d.ts",
+  "main": "./types/types.js"


I see we have the path specified here. So if we ever move it around in the repo, we can just update it here 👍

ruflin · 2020-04-27T12:39:14Z

@jfsiii What should we do with this PR? Should we close it for now or get it in?

ph · 2020-05-25T15:29:41Z

@jfsiii @ruflin should we close this?

ruflin · 2020-05-26T06:02:10Z

This ties into elastic/kibana#54955 (comment) Same suggestion, close it but have an issue in Kibana that tracks this idea and also references both PR's.

ruflin · 2020-06-10T12:21:49Z

Closing this PR for now. I think #490 will also be helpful here.

Create TS types in mage build

7189569

Use github.com/OneOfOne/struct2ts to generate `types.d.ts` file from `util.Package` struct.

jfsiii requested a review from ruflin January 15, 2020 19:31

jfsiii commented Jan 15, 2020

View reviewed changes

jfsiii changed the title ~~Create TS types in mage build #195~~ Create TS types in mage build closes #195 Jan 15, 2020

jfsiii changed the title ~~Create TS types in mage build closes #195~~ Create TS types in mage build Jan 15, 2020

jfsiii mentioned this pull request Jan 15, 2020

[EPM] Consume TS types from EPR. Update EPM app code. elastic/kibana#54955

Closed

ruflin reviewed Jan 16, 2020

View reviewed changes

ruflin assigned jfsiii Jan 16, 2020

John Schulz added 2 commits January 16, 2020 08:46

Move types to docs/ directory

8ff3676

Put typings under new types directory

dd4785a

ruflin approved these changes Jan 20, 2020

View reviewed changes

ruflin unassigned jfsiii Apr 9, 2020

jfsiii mentioned this pull request Jun 1, 2020

[Discuss] OpenAPI spec for registry #487

Open

ruflin closed this Jun 10, 2020

		@@ -0,0 +1,59 @@
		// this file was automatically generated, DO NOT EDIT

Conversation

jfsiii commented Jan 15, 2020 • edited by ruflin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

ruflin left a comment

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!

ruflin left a comment

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!

ruflin commented Apr 27, 2020

Uh oh!

ph commented May 25, 2020

Uh oh!

ruflin commented May 26, 2020

Uh oh!

ruflin commented Jun 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jfsiii commented Jan 15, 2020 •

edited by ruflin

Loading