Packaging & publishing to npm #202

mishig25 · 2021-07-16T10:55:21Z

Update: 2021-09-09

@Pierrci @beurkinger
Since internal was merged, I've advanced this PR.
To remind:

This PR published widgets as a npm package https://www.npmjs.com/package/huggingface-widgets
Removed the $lib usage since Resolve aliases while running svelte-kit package sveltejs/kit#1950 is still open. (You mentioned about it here)
Files that are being export are:

".": "./InferenceWidget/InferenceWidget.svelte",
"./WidgetHeader.svelte": "./InferenceWidget/shared/WidgetHeader/WidgetHeader.svelte",
"./ModelPipelineIcon.svelte": "./ModelPipelineIcon/ModelPipelineIcon.svelte",
"./ModelPipelineTag.svelte": "./ModelPipelineTag/ModelPipelineTag.svelte",
"./inferenceSnippets": "./inferenceSnippets/index.js",
"./inferenceSnippets/inputs": "./inferenceSnippets/inputs.js",
"./interfaces/DefaultWidget": "./interfaces/DefaultWidget.js",
"./interfaces/Language": "./interfaces/Language.js",
"./interfaces/Libraries": "./interfaces/Libraries.js",
"./interfaces/Types": "./interfaces/Types.js"

And it gets consumed as:

import InferenceWidget from "huggingface-widgets";
import ModelPipelineTag from "huggingface-widgets/ModelPipelineTag.svelte";
import { Language } from "huggingface-widgets/interfaces/Language";
import type { PipelineType } from "huggingface-widgets/interfaces/Types";
...

Checkout its companion PR internally here

Pierrci · 2021-07-16T15:17:10Z

Did you close this accidentally @mishig25 or was it intended?

mishig25 · 2021-07-16T15:37:28Z

@Pierrci Yeah it was intended. I think changes I've made to tsconfig is causing svelte-check to fail

mishig25 · 2021-07-16T15:39:09Z

@Pierrci Please let me know if you think this approach is acceptable.

Why we can't use a package generated by `svelte-kit package`?

svelte-kit package expects the consumer (in this case, moon-landing) to be using svelte-kit, which directly consumes third party component sources (rather than bundled files).
However, on moon-landing, we don't use svelte-kit. There are 2 ways a component gets compiled on moon-landing: build-svelte.mjs on server and rollup on front (rollup can consume svelte component sources directly, so we can use svelte-kit package for the front (not server) see here).

As a intermediary solution

I've copied moon-landing/server/build-svelte.mjs into hub/widgets/build.js. In hub/widgets/package folder:

a component source code is copied here from hub/widgets/package(withsvelte-kit packageformoon-landing/front`)
a component gets compiled here with hub/widgets/build.js (for moon-landing/server)

And npm package is published.

I think this approach is quite "hacky". As we add more svelte components, we should definitely switch to what @beurkinger suggested.

julien-c · 2021-07-20T18:26:35Z

nit, but the npm package name (if we go that way) would be better named huggingface-widgets for consistency w/ other library etc. naming

Pierrci · 2021-07-22T22:30:50Z

I finally had a look at it, and IMO it looks very promising @mishig25 :)

I think changes I've made to tsconfig is causing svelte-check to fail

I bumped the version of svelte-check to the last one locally (2.2.3) and it seems to be working fine, it was probably linked to some bug :)

I think this approach is quite "hacky". As we add more svelte components, we should definitely switch to what @beurkinger suggested.

I agree the approach can be considered as "hacky", but I'm not sure why adding more components would make it worse than it is now?

More generally, I think we could refactor a bit the js structure in this repo w/ a more generic js (or similar) top-level directory instead of widgets, move interfaces in lib and publish a more generic huggingface(-hub?) package exposing both the widgets through InferenceWidget and what would be in lib/interfaces. (+ ModelPipelineIcon and ModelPipelineTag since we also use them in moon-landing). wdyt @julien-c?

This way we can import all the js code we need from huggingface_hub in a unified way (and not using the npm package in some places and weird paths pointing to the submodule in others).

mishig25 · 2021-07-23T12:00:21Z

I made an ambiguous statement above. I meant: we should switch to rollup if/when we add third-party svelte component libraries.

but I'm not sure why adding more components would make it worse than it is now?

julien-c · 2021-07-23T16:31:25Z

More generally, I think we could refactor a bit the js structure in this repo w/ a more generic js (or similar) top-level directory instead of widgets, move interfaces in lib and publish a more generic huggingface(-hub?) package exposing both the widgets through InferenceWidget and what would be in lib/interfaces. (+ ModelPipelineIcon and ModelPipelineTag since we also use them in moon-landing). wdyt @julien-c?

No strong opinion (I think git-submodules are fine for non-Svelte files), I'll leave that up to you guys.

Pierrci · 2021-07-23T22:35:50Z

No strong opinion (I think git-submodules are fine for non-Svelte files), I'll leave that up to you guys.

Yes, we can leave the hub doc outside of the package 😄. But moving interfaces would also simplify both their use inside the widgets and the package's build (@mishig25 had to add a specific build step because of their current path). What do you think of the idea @mishig25?

@LysandreJik, what do you think would be the best way to regroup all the js code (widgets + interfaces - if we go this way) without interfering too much with all the pythonic stuff? something inside src? next to it?

julien-c · 2021-07-24T06:53:43Z

What about keeping interfaces/ and nesting inside interfaces/widgets/?

LysandreJik · 2021-07-24T15:29:54Z

I'd rather we don't move it inside src as that's the current location of the python source code and is what we use in the setup.py to create the wheels.

Moving widgets under interfaces/widgets is ok for me. Moving both folders under an unambiguous folder frontend or hub is also good for me if you'd like to keep the same structure.

mishig25 · 2021-07-24T16:56:37Z

Moving both folders under an unambiguous folder frontend

sounds great to me.

In that case, @Pierrci, could we just add huggingface_hub/frontend dir to tsconfig files & build-svelte.mjs script and not use rollup at the moment? And use tsconfig#paths so that we don't use really long import statements like import huggingface_hub/frontend/widgets/src/lib/InferenceWidget?

julien-c · 2021-07-26T07:24:26Z

I'll reiterate that I'd prefer to nest widgets under interfaces/ given that we already have a interfaces/ directory (and its contents does not need to be moved unless I'm mistaken)

mishig25 · 2021-07-26T07:34:44Z

got it 👍
then, I'll move widgets inside interfaces as interfaces/widgets/

Pierrci · 2021-07-26T22:16:28Z

What benefits do you see @julien-c in keeping interfaces and move widgets in it?

Moving interface inside widgets/lib would simplify a lot the build setup (from my tests we can remove the second half of the steps - maybe more), but the opposite doesn't change anything, except a few relative paths that lose a /interface in them...

(w/ the current setup - same w/ simply moving widgets in interfaces - even the basic svelte-kit package needs tweaking b/c what's in interfaces can't be imported properly, and typings generation is a mess)

julien-c · 2021-07-27T07:31:31Z

Ok ok, I haven't tested the whole tooling so let's go with what @Pierrci preconizes. 🙏

mishig25 · 2021-07-29T07:47:58Z

@Pierrci in terms for publishing the widgets, what should be the next step:
Option A: use the build script plus svelte-kit package method suggested by this PR
Option B: something else, as referred:

(from my tests we can remove the second half of the steps - maybe more

Pierrci · 2021-07-29T16:30:29Z

@mishig25 option A, but following #227 we can simplify what's done in this PR and remove a few steps from the build.js file since you'll find that moving interfaces inside lib/ makes the svelte-kit package command runs more as one would expect (files aren't outputted in 2 different dirs inside package/ anymore for example).

Ideally we would end up exposing InferenceWidget, ModelPipelineTag, ModelPipelineTag and WidgetHeader from the "root" of the package, and everything that's in interfaces from huggingface-widgets/interfaces? You'll probably need to create one root index.ts and another one in interfaces (they talk about creating an index.ts or index.svelte in https://kit.svelte.dev/docs#packaging, but it's a pretty common way of packaging anyway).

widgets/package.json

widgets/build.js

widgets/package.json

Pierrci · 2021-08-02T16:57:08Z

widgets/tsconfig.svelte.json

+		"strictFunctionTypes": true,
+		"strictNullChecks": true,
+		"strictBindCallApply": true,
+		"lib": ["es6", "es2016", "es2017", "es2018", "esnext"]


I think the lib entry can be removed here no? (see https://www.typescriptlang.org/tsconfig#target: "Changing target also changes the default value of lib.")

mishig25 · 2021-09-09T10:31:32Z

@Pierrci @beurkinger
Since internal was merged, I've advanced this PR.
To remind:

This PR published widgets as a npm package https://www.npmjs.com/package/huggingface-widgets
Removed the $lib usage since Resolve aliases while running svelte-kit package sveltejs/kit#1950 is still open. (You mentioned about it here)
Files that are being export are:

".": "./InferenceWidget/InferenceWidget.svelte",
"./WidgetHeader.svelte": "./InferenceWidget/shared/WidgetHeader/WidgetHeader.svelte",
"./ModelPipelineIcon.svelte": "./ModelPipelineIcon/ModelPipelineIcon.svelte",
"./ModelPipelineTag.svelte": "./ModelPipelineTag/ModelPipelineTag.svelte",
"./inferenceSnippets": "./inferenceSnippets/index.js",
"./inferenceSnippets/inputs": "./inferenceSnippets/inputs.js",
"./interfaces/DefaultWidget": "./interfaces/DefaultWidget.js",
"./interfaces/Language": "./interfaces/Language.js",
"./interfaces/Libraries": "./interfaces/Libraries.js",
"./interfaces/Types": "./interfaces/Types.js"

And it gets consumed as:

import InferenceWidget from "huggingface-widgets";
import ModelPipelineTag from "huggingface-widgets/ModelPipelineTag.svelte";
import { Language } from "huggingface-widgets/interfaces/Language";
import type { PipelineType } from "huggingface-widgets/interfaces/Types";
...

Checkout its companion PR internally here

Pierrci

We're almost there! :)

Pierrci · 2021-09-13T10:36:36Z

widgets/src/routes/[...model].svelte

+	import type { ModelData } from "../../src/lib/interfaces/Types";

-	import InferenceWidget from "$lib/InferenceWidget/InferenceWidget.svelte";
+	import InferenceWidget from "../../src/lib/InferenceWidget/InferenceWidget.svelte";


why not ../lib directly here?

Pierrci · 2021-09-14T09:58:46Z

widgets/postpackage.js

+delete pkg.type;
+fs.writeFileSync(pathtToJson, JSON.stringify(pkg));
+
+copyRecursiveSync("../docs", "./package/docs");


As discussed in https://github.com/huggingface/moon-landing/pull/1172, let's keep the docs separate, at least for now.

Pierrci · 2021-09-17T15:46:15Z

widgets/package.json

+		"./interfaces/DefaultWidget": "./interfaces/DefaultWidget.js",
+		"./interfaces/Language": "./interfaces/Language.js",
+		"./interfaces/Libraries": "./interfaces/Libraries.js",
+		"./interfaces/Types": "./interfaces/Types.js"


Can we have an interfaces/index.js that re-exports those rather, so that we can do import { DefaultWidget, Language, ... } from "huggingface-widgets/interfaces"?

Does it make sense to do so? because, the use case has always been:
import { x, y, z } from Interfaces or Language,
not import Interfaces

Yes, you're right actually, let's keep it that way! I think I'm feeling the effects of the jetlag x)

Pierrci · 2021-09-17T15:50:19Z

widgets/package.json

-	"version": "0.0.1",
+	"version": "0.15.0",
+	"type": "module",
+	"main": "./InferenceWidget/InferenceWidget.svelte",


main is supposed to be something in plain js I think (that's why you can add a specific svelte entry for the "non-compiled" version), but for now, we can leave it as is - and add a "compiled" version later if there is a demand for it.

Pierrci · 2021-09-17T15:51:05Z

widgets/package.json

@@ -1,10 +1,23 @@
 {
 	"name": "huggingface-widgets",
-	"version": "0.0.1",
+	"version": "0.15.0",


What about bumping the version to 1.0.0 when we release for good? :)

mishig25 · 2021-10-11T10:54:05Z

Closing it for now

mishig25 added 2 commits July 16, 2021 11:49

Add buil script & update readme

c2b1e54

Merge branch 'main' into packaging

f143c73

mishig25 requested review from julien-c, Pierrci and beurkinger and removed request for julien-c, Pierrci and beurkinger July 16, 2021 10:55

mishig25 closed this Jul 16, 2021

mishig25 reopened this Jul 22, 2021

mishig25 mentioned this pull request Jul 26, 2021

Mv widgets -> interfaces/widgets #223

Closed

mishig25 mentioned this pull request Jul 27, 2021

Mv interfaces -> widgets/lib/interfaces #227

Merged

Merge branch 'main' into packaging

d76a6c1

mishig25 added 10 commits July 30, 2021 08:49

Rm unnecessary build lines

6a361ed

Apply path aliases

338e79c

Update svelte-check

3741efe

Chore tsconfig

446d5d5

Export ModelPipelineIcon & ModelPipelineTag

9aaa54b

Publish only package folder

2dea8b7

Add interface export

600dbd1

Update readme

d4fdc25

Merge branch 'main' into packaging

5fec184

Fix svelte exports

2e791b4

mishig25 commented Jul 30, 2021

View reviewed changes

widgets/package.json Outdated Show resolved Hide resolved

mishig25 marked this pull request as ready for review July 30, 2021 15:51

mishig25 requested a review from Pierrci July 30, 2021 15:51

Pierrci reviewed Aug 3, 2021

View reviewed changes

mishig25 added 10 commits August 10, 2021 15:57

Merge branch 'main' into packaging

3c5326a

Rm $lib path

a7fc371

Merge branch 'main' into packaging

c217710

Chore

2c08231

Add postpackage.json

42c6a21

Module exorts

000b9ee

Lint & check

b9ed3f6

Export widgetheader

185b204

Export docs

5b8492e

Rm unnecessary exports

585288a

Pierrci reviewed Sep 17, 2021

View reviewed changes

mishig25 closed this Oct 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Packaging & publishing to npm #202

Packaging & publishing to npm #202

mishig25 commented Jul 16, 2021 •

edited

Loading

Pierrci commented Jul 16, 2021

mishig25 commented Jul 16, 2021

mishig25 commented Jul 16, 2021 •

edited

Loading

julien-c commented Jul 20, 2021

Pierrci commented Jul 22, 2021

mishig25 commented Jul 23, 2021 •

edited

Loading

julien-c commented Jul 23, 2021

Pierrci commented Jul 23, 2021

julien-c commented Jul 24, 2021 •

edited

Loading

LysandreJik commented Jul 24, 2021

mishig25 commented Jul 24, 2021 •

edited

Loading

julien-c commented Jul 26, 2021

mishig25 commented Jul 26, 2021

Pierrci commented Jul 26, 2021 •

edited

Loading

julien-c commented Jul 27, 2021

mishig25 commented Jul 29, 2021

Pierrci commented Jul 29, 2021

Pierrci Aug 2, 2021

mishig25 commented Sep 9, 2021 •

edited

Loading

Pierrci left a comment

Pierrci Sep 13, 2021

Pierrci Sep 14, 2021

Pierrci Sep 17, 2021 •

edited

Loading

mishig25 Sep 17, 2021

Pierrci Sep 17, 2021 •

edited

Loading

Pierrci Sep 17, 2021

Pierrci Sep 17, 2021

mishig25 Sep 17, 2021

mishig25 commented Oct 11, 2021

Packaging & publishing to npm #202

Packaging & publishing to npm #202

Conversation

mishig25 commented Jul 16, 2021 • edited Loading

Pierrci commented Jul 16, 2021

mishig25 commented Jul 16, 2021

mishig25 commented Jul 16, 2021 • edited Loading

Why we can't use a package generated by svelte-kit package?

As a intermediary solution

julien-c commented Jul 20, 2021

Pierrci commented Jul 22, 2021

mishig25 commented Jul 23, 2021 • edited Loading

julien-c commented Jul 23, 2021

Pierrci commented Jul 23, 2021

julien-c commented Jul 24, 2021 • edited Loading

LysandreJik commented Jul 24, 2021

mishig25 commented Jul 24, 2021 • edited Loading

julien-c commented Jul 26, 2021

mishig25 commented Jul 26, 2021

Pierrci commented Jul 26, 2021 • edited Loading

julien-c commented Jul 27, 2021

mishig25 commented Jul 29, 2021

Pierrci commented Jul 29, 2021

Pierrci Aug 2, 2021

Choose a reason for hiding this comment

mishig25 commented Sep 9, 2021 • edited Loading

Pierrci left a comment

Choose a reason for hiding this comment

Pierrci Sep 13, 2021

Choose a reason for hiding this comment

Pierrci Sep 14, 2021

Choose a reason for hiding this comment

Pierrci Sep 17, 2021 • edited Loading

Choose a reason for hiding this comment

mishig25 Sep 17, 2021

Choose a reason for hiding this comment

Pierrci Sep 17, 2021 • edited Loading

Choose a reason for hiding this comment

Pierrci Sep 17, 2021

Choose a reason for hiding this comment

Pierrci Sep 17, 2021

Choose a reason for hiding this comment

mishig25 Sep 17, 2021

Choose a reason for hiding this comment

mishig25 commented Oct 11, 2021

mishig25 commented Jul 16, 2021 •

edited

Loading

mishig25 commented Jul 16, 2021 •

edited

Loading

Why we can't use a package generated by `svelte-kit package`?

mishig25 commented Jul 23, 2021 •

edited

Loading

julien-c commented Jul 24, 2021 •

edited

Loading

mishig25 commented Jul 24, 2021 •

edited

Loading

Pierrci commented Jul 26, 2021 •

edited

Loading

mishig25 commented Sep 9, 2021 •

edited

Loading

Pierrci Sep 17, 2021 •

edited

Loading

Pierrci Sep 17, 2021 •

edited

Loading