search

cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
27 stars 23 forks source link

Use `import type` where appropriate #596

Closed jelly closed 1 day ago

jelly commented 3 weeks ago

This makes type imports not end up in JavaScript if they are unneeded (after transformation) this likely won't lead to a shocking reduction in bundle size.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html

allisonkarlitskaya commented 3 weeks ago

Would be great if we could get an eslint rule for this.

Would also be great if we got our eslint setup working properly...

allisonkarlitskaya commented 3 weeks ago

I'll copy my comment from https://github.com/cockpit-project/cockpit/pull/20648#discussion_r1652138573 because I think it's appropriate (although who knows — maybe I'm wrong).

I think import type only makes sense on files that you wouldn't already import for non-type reasons.

ie: If you already have an import then don't add an import type. But if you wouldn't otherwise have an import, and only want it for typing reasons, then import type is the one you want.

Patrick-1-1-1-PH commented 3 days ago

:~/cockpit-files/cockpit-files$ make remote: Enumerating objects: 1533, done. remote: Counting objects: 100% (1533/1533), done. remote: Compressing objects: 100% (1358/1358), done. remote: Total 1533 (delta 251), reused 601 (delta 88), pack-reused 0 Receiving objects: 100% (1533/1533), 7.49 MiB | 6.58 MiB/s, done. Resolving deltas: 100% (251/251), done. git archive '319e2d4539f183dc0914cc1903a09df211d8d73f^{tree}' -- pkg/lib test/common test/static-code tools/node-modules | tar x make package-lock.json && NODE_ENV= ./build.js make[1]: Entering directory '/home/orangepi/cockpit-files/cockpit-files' tools/node-modules make_package_lock_json REMOVE node_modules CLONE node_modules [ref: c66251b62545483104299f149b86fc37b9a1ee8a] COPY package-lock.json make[1]: Leaving directory '/home/orangepi/cockpit-files/cockpit-files' node: bad option: --import make: *** [Makefile:93: dist/manifest.json] Error 9

There is an error in import in a node command. I think this is what it refered to.

jelly commented 1 day ago

@Patrick-1-1-1-PH what node version do you use? Although this issue is unrelated to the type import issue so please open a new issue if you have issues building files.