Add ladle plugin #728

justb3a · 2024-07-12T13:33:27Z

Ladle is a lightweight Storybook alternative. Its configuration is stored in .ladle/config.mjs, along with some other files. It uses Vite, a custom Vite config is sometimes necessary in a customizable location via viteConfig.

Related issue: #268

netlify · 2024-07-12T13:33:30Z

✅ Deploy Preview for knip canceled.

Name	Link
🔨 Latest commit	`9ca4e9a`
🔍 Latest deploy log	https://app.netlify.com/sites/knip/deploys/669e8f9021796b0008a8be98

webpro

Thanks @justb3a! A Ladle plugin is a great addition to Knip.

My main point of feedback is that the fixtures are a bit minimal. Ideally there would be more configuration with stories and components to verify the right files are indeed used as entry files (and they'll probably use dependencies that are then no longer unused).

webpro · 2024-07-13T10:09:45Z

packages/knip/src/plugins/ladle/index.ts

+
+const resolveEntryPaths: ResolveEntryPaths<LadleConfig> = async localConfig => {
+  const localStories = typeof localConfig.stories === 'string' ? [localConfig.stories] : localConfig.stories;
+  const localViteConfig = localConfig.viteConfig ? [localConfig.viteConfig] : [];


The Vite config should be handled by the Vite plugin, so I think we can just ignore it here? Btw it would need be a config file, not an entry file.

For ladle it might be necessary to have a dedicated / different viteConfig file. So in my case I have ~/.vite.config.ts and for ladle ~/.ladle/vite.config.ts. Only if you don't use the default one, this parameter is set and then should be respected. Otherwise, I get it listed as "unused file".

OK in that case I think it's interesting to look into using the resolveConfig function of the Vitest plugin (which is basically the same as the Vite plugin).

Simplified inside resolveConfig: const viteDeps = resolveConfig(await load(localConfig.viteConfig))

Does that make sense?

Changed 👍 I wasn't aware that it is allowed to use other plugins.

… I cannot get rid of the message that the file is unused, stuck here.

Unused files (1) .ladle/vite.config.ts

I also tried things like

const resolveConfig: ResolveConfig<LadleConfig> = async (localConfig, options) => resolveVitestConfig(localConfig.viteConfig ? await load(join(process.cwd(), localConfig.viteConfig)) : [], options);

Maybe someone with more insights can help here? 😇

Perhaps confusingly, loading the file does not "use" it (because it's not part of the dependency graph). For this you could add it as an entry file (contrary to what I said earlier, sorry, but that was for another reason).

I re-added it additionally as an entry file 👍

packages/knip/src/plugins/ladle/index.ts

pkg-pr-new · 2024-07-13T10:22:24Z

Run & review this pull request in StackBlitz Codeflow.

commit: 9ca4e9a

knip

npm i https://pkg.pr.new/knip@728

templates

default

justb3a · 2024-07-15T09:44:03Z

@webpro thanks for the feedback. I added some commits on top and extended the fixtures. If you want to have something else regarding fixtures/tests pls give me some hints, I checked some other plugins and couldn't find something helpful. I did not add node_modules folder because this one would be quite big even with the limited set of package json entries.

webpro · 2024-07-15T10:35:41Z

I added some commits on top and extended the fixtures. If you want to have something else regarding fixtures/tests pls give me some hints, I checked some other plugins and couldn't find something helpful.

A Ladle config file
You mentioned a Vite config file (did you see Add ladle plugin #728 (comment)?)
There should be a .ladle directory with specific files?

Most plugins should have fixtures for the config and entry files they're referencing in the implementation.

I did not add node_modules folder because this one would be quite big even with the limited set of package json entries.

Great, that would be too much indeed. Sometimes a basic definConfig might come in useful, such as the one for Vite (https://github.com/webpro-nl/knip/blob/main/packages/knip/fixtures/plugins/vite/node_modules/vite/index.js).

webpro · 2024-07-15T14:01:11Z

packages/knip/src/plugins/ladle/index.ts

+};
+
+const resolveConfig: ResolveConfig<LadleConfig> = async (localConfig, options) =>
+  localConfig.viteConfig ? resolveVitestConfig(await load(localConfig.viteConfig), options) : [];


Please expand it a bit and use something like join(options.configFileDir, localConfig.viteConfig) so that it is resolved relative the Ladle config file dir.

Oh never mind, now I read https://ladle.dev/docs/config/#viteconfig and see that the path should be absolute anyway (wasn't clear from the fixture).

Not sure, just the example uses an absolute path, I could not find something in the documentation stating that is has to be absolute.

I think it would be great to support both, so I now check if the path is already absolute and if not, I join it with options.cwd. (I did not check before what's inside options, so helpful 🤣 ).

At least on my machine with our config it now works like a charm :)

justb3a · 2024-07-16T11:45:08Z

@webpro do you have any idea why the test is failing in CI?

test/plugins/ladle.test.ts:
  dyld[2233]: missing symbol called
  error: script "test" was terminated by signal SIGABRT (Abort)
  /Users/runner/work/_temp/e765fe65-ff66-46a8-84ea-064701d7180d.sh: line 1:  2232 Abort trap: 6           bun run test
  Error: Process completed with exit code 134.

locally it's working

❯ bun test test/plugins/ladle.test.ts
bun test v1.1.18 (5a0b9352)

test/plugins/ladle.test.ts:
✓ Find dependencies with the ladle plugin [135.67ms]

 1 pass
 0 fail
Ran 1 tests across 1 files. [473.00ms]

webpro · 2024-07-16T11:47:34Z

Yeah, crazy. I'll take a look tonight.

webpro · 2024-07-17T17:35:49Z

The issue seems to be with Bun 1.1.19, 1.1.20 handles it fine.

webpro · 2024-07-17T17:54:47Z

But bun 1.1.20 has another issue, so I'm afraid we'll have to wait for a fix. Still no idea what eventually causes the issue in this PR 🤷‍♂️

alecmev · 2024-07-23T10:20:46Z

This is great, thanks! 👍

webpro · 2024-07-23T10:44:15Z

Thanks to @justb3a! There was an issue with the release script, apparently the release notes were too long, but this PR is part of v5.27.0.

Add ladle plugin

733e0a7

webpro reviewed Jul 13, 2024

View reviewed changes

justb3a added 4 commits July 15, 2024 10:37

Remove head.html from entry files since it is not supported

8beeb70

Use same defaults as ladle does

66f9edb

Handle specific vite config as config file

1bfc42d

Extend ladle fixtures

4025e7f

justb3a added 2 commits July 15, 2024 15:24

Use vitest resolve config

c813e69

Add custom .ladle config

67c4e87

justb3a force-pushed the feat/plugin-ladle branch from 1e7bb4b to 67c4e87 Compare July 15, 2024 13:24

webpro reviewed Jul 15, 2024

View reviewed changes

justb3a added 3 commits July 16, 2024 08:36

Fix local vite config path

cfefa56

Fix ladle tests

0f5ae20

Organize imports

1bf1838

Minor refactor ladle plugin

c8c051e

webpro added 2 commits July 17, 2024 19:35

Merge branch 'main' into feat/plugin-ladle

452c099

Try bun 1.1.20

00c7af2

webpro added 3 commits July 22, 2024 18:50

Minor refactor

fff5023

Try latest bun

52b34c7

Add upgrade bun command to binary resolver

9ca4e9a

webpro merged commit ebd79d4 into webpro-nl:main Jul 22, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ladle plugin #728

Add ladle plugin #728

justb3a commented Jul 12, 2024

netlify bot commented Jul 12, 2024 •

edited

Loading

webpro left a comment

webpro Jul 13, 2024

justb3a Jul 15, 2024

webpro Jul 15, 2024

justb3a Jul 15, 2024

justb3a Jul 15, 2024

webpro Jul 15, 2024

justb3a Jul 16, 2024

pkg-pr-new bot commented Jul 13, 2024 •

edited

Loading

justb3a commented Jul 15, 2024

webpro commented Jul 15, 2024

webpro Jul 15, 2024

webpro Jul 15, 2024

justb3a Jul 16, 2024

justb3a commented Jul 16, 2024

webpro commented Jul 16, 2024

webpro commented Jul 17, 2024

webpro commented Jul 17, 2024

alecmev commented Jul 23, 2024

webpro commented Jul 23, 2024

Add ladle plugin #728

Add ladle plugin #728

Conversation

justb3a commented Jul 12, 2024

netlify bot commented Jul 12, 2024 • edited Loading

✅ Deploy Preview for knip canceled.

webpro left a 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

Choose a reason for hiding this comment

pkg-pr-new bot commented Jul 13, 2024 • edited Loading

justb3a commented Jul 15, 2024

webpro commented Jul 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justb3a commented Jul 16, 2024

webpro commented Jul 16, 2024

webpro commented Jul 17, 2024

webpro commented Jul 17, 2024

alecmev commented Jul 23, 2024

webpro commented Jul 23, 2024

netlify bot commented Jul 12, 2024 •

edited

Loading

pkg-pr-new bot commented Jul 13, 2024 •

edited

Loading