search

shellscape / webpack-manifest-plugin

webpack plugin for generating asset manifests
MIT License
1.43k stars 185 forks source link

Types of `generate` function don't match the docs #276

Closed sapkra closed 2 years ago

sapkra commented 3 years ago

Expected Behavior

It should be possible to return a nested javascript Object in the generate function, as documented:

A custom Function to create the manifest. The passed function should match the signature of (seed: Object, files: FileDescriptor[], entries: string[]) => Object and can return anything as long as it's serialisable by JSON.stringify.

Actual Behavior

One accepts an Object with string values.

Additional Information

https://github.com/shellscape/webpack-manifest-plugin/blob/77eca2545b70ea235fc2c6b5ddf0f90540dd8588/src/index.ts#L13

https://github.com/shellscape/webpack-manifest-plugin/blob/77eca2545b70ea235fc2c6b5ddf0f90540dd8588/src/index.ts#L20-L24

The code doesn't match the docs. I think the types are wrong but prove me wrong.

shellscape commented 3 years ago

Issue templates are required here. Please fill one out and update your issue, and we'll reopen.

sapkra commented 3 years ago

@shellscape I created this issue by referencing code, so there was no hint regarding templates. I updated it now.

shellscape commented 3 years ago

Thanks. Will take a look as time allows.

KutnerUri commented 2 years ago

ah yes, it's also confusing. I've put an unnecessary JSON.stringify() there, and then it wouldn't parse correctly. Type should be JSON, you can use: https://www.npmjs.com/package/@teambit/base-ui.utils.primitives

or 

export type Nullable = undefined | null;
export type Primitive = string | number | bigint | boolean | Nullable;
export type JSON = { [key: string]: JSON |  Primitive | (Primitive | JSON)[] };
hagevvashi commented 2 years ago

@shellscape

Cloud you change the type Manifest to Record<string, unknown> ?

I think that this issue is related to below.

https://github.com/shellscape/webpack-manifest-plugin/issues/278

stale[bot] commented 2 years ago

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ

KutnerUri commented 2 years ago

this issue is resolved by #282. @shellscape - can you merge? :D