Skip to content

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Oct 28, 2024

Fixes #1951

The reason why type definitions were not being published was that lerna was using ./packages/playground/remote/ as the source directory, instead of ./dist/packages/playground/remote/. Since the type definitions are only present in dist/, they were not being published.

In #1924 I did test that the published package contained the type definitions, but I did not test it using lerna. Instead, I tested using npm publish directly from the dist/ directory. Now I have tested this PR using lerna and can confirm that the published package will contain the type definitions.

I learned my lesson, from now on I'll always test publishing packages with lerna instead of npm publish :)

Here's the relevant lerna logs that show the type definitions will now be included in the published package:

lerna notice 📦 @wp-playground/remote@1.0.8 lerna notice === Tarball Contents === lerna notice 696B package.json lerna notice 465B lib/boot-playground-remote.d.ts lerna notice 92B lib/config.d.ts lerna notice 567B lib/create-memoized-fetch.d.ts lerna notice 23B index.d.ts lerna notice 220B lib/index.d.ts lerna notice 587B lib/progress-bar/index.d.ts lerna notice 1.4kB lib/offline-mode-cache.d.ts lerna notice 2.4kB lib/playground-client.d.ts lerna notice 677B lib/setup-fetch-network-transport.d.ts lerna notice 2.4kB lib/worker-thread.d.ts lerna notice 3.6kB lib/worker-utils.d.ts lerna notice === Tarball Details === 
@psrpinto psrpinto self-assigned this Oct 28, 2024
@psrpinto psrpinto force-pushed the fix-remote-missing-files branch 4 times, most recently from e55b046 to a791708 Compare October 28, 2024 18:52
@psrpinto psrpinto force-pushed the fix-remote-missing-files branch from a791708 to 41b3102 Compare October 29, 2024 11:25
@psrpinto psrpinto marked this pull request as ready for review October 29, 2024 11:41
@psrpinto psrpinto requested review from adamziel and bgrgicak October 29, 2024 11:41
@bgrgicak
Copy link
Collaborator

I learned my lesson, from now on I'll always test publishing packages with lerna instead of npm publish :)

I learned something new. 😄

@bgrgicak
Copy link
Collaborator

The changes look good to me.

@psrpinto would you mind sharing the full testing instructions?

@bgrgicak bgrgicak added [Type] Bug An existing feature does not function as intended [Package][@wp-playground] Remote labels Oct 30, 2024
@adamziel
Copy link
Collaborator

I learned my lesson, from now on I'll always test publishing packages with lerna instead of npm publish :)

Here's the relevant lerna logs that show the type definitions will now be included in the published package:

Test-publishing npm packages sounds really useful. Would you please document your process in some place the next person would likely look at? One option would be contributor docs, another would be contextually somewhere you have to look before publishing (I don't have any specific location in mind).

@adamziel adamziel changed the title [Remote] Fix missing files in published package Restore .d.ts files missing from the published @wp-playground/remote npm package Oct 30, 2024
@adamziel adamziel merged commit 9cbdd09 into trunk Oct 30, 2024
10 checks passed
@adamziel adamziel deleted the fix-remote-missing-files branch October 30, 2024 09:42
@psrpinto
Copy link
Member Author

would you mind sharing the full testing instructions?

Sorry for not providing full test instructions, I will make sure to do so in future PRs.

Part of the reason I didn't share the test instructions was that the procedure to test publishing packages locally is not straightforward, so I thought "no one is actually gonna follow these instructions" :D (In the description of #1924 you can see more or less what is involved, though I did manage to simplify it a bit.)

Test-publishing npm packages sounds really useful. Would you please document your process in some place the next person would likely look at?

I would be happy to document the process. Incidentally, I noticed that lerna also uses verdaccio:

I will take inspiration from lerna's docs, and document something similar for playground.

@adamziel
Copy link
Collaborator

adamziel commented Oct 30, 2024

no one is actually gonna follow these instructions

Even if no one followed them, they'd be super helpful when preparing the next similar PR. Perhaps you would even come back to this PR at one point thinking oh, I've done that once and I think I documented the testing steps somewhere – that happens to me all the time :)

@psrpinto
Copy link
Member Author

Great point Adam, writing documentation for oneself makes a lot of sense.

@brandonpayton
Copy link
Member

Even if no one followed them, they'd be super helpful when preparing the next similar PR. Perhaps you would even come back to this PR at one point thinking oh, I've done that once and I think I documented the testing steps somewhere – that happens to me all the time :)

I love this so much. Everybody wins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package][@wp-playground] Remote [Type] Bug An existing feature does not function as intended

4 participants