Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publish @wp-playground/remote (types only) #1924

Merged
merged 8 commits into from
Oct 25, 2024
Merged

Publish @wp-playground/remote (types only) #1924

merged 8 commits into from
Oct 25, 2024

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Oct 19, 2024

Fixes #1725

This PR publishes the @wp-playground/remote package with only type definitions, as per discussion in #1725.

I've tested the changes introduced here locally, using verdaccio, and can confirm that the issues with missing types (namely PlaygroundClient) are fixed.

I took the liberty to fix some small things I noticed, and made sure to add context in commit messages, when necessary.

Package contents

.
├── lib
│   ├── progress-bar
│   │   └── index.d.ts
│   ├── boot-playground-remote.d.ts
│   ├── config.d.ts
│   ├── create-memoized-fetch.d.ts
│   ├── index.d.ts
│   ├── offline-mode-cache.d.ts
│   ├── playground-client.d.ts
│   ├── setup-fetch-network-transport.d.ts
│   ├── worker-thread.d.ts
│   └── worker-utils.d.ts
├── index.d.ts
└── package.json

Testing instructions

Install verdaccio:

npm install -g verdaccio

Run it:

verdaccio

There would probably be a smarter way to test this without modifying the files below, but this got the job done for me.

Edit tools/scripts/publish.mjs so that the spawnSync call takes an extra --registry argument:

spawnSync('npm', ['publish', '--access', 'public', tag ? `--tag ${tag}` : '', '--registry', 'http://localhost:4873/'], {

Edit the publish command in packages/playground/client/project.json so that it looks something like:

"command": "node tools/scripts/publish.mjs playground-client 2.0.0 test",

Edit the publish command in packages/playground/remote/project.json so that it looks something like:

"command": "node tools/scripts/publish.mjs playground-remote 0.0.1 test",

Publish both packages to verdaccio:

npx nx publish playground-remote
npx nx publish playground-client

Clone the test repo and configure it to use the verdaccio registry:

cd some-dir
git clone [email protected]:psrpinto/playground-ts-test.git
cd playground-ts-test

# Will create a .npmrc file in the current directory. 
npm set registry http://localhost:4873/ --location project

Edit package.json of the test repo so that it references the version of @wp-playground/client you published above:

"@wp-playground/client": "^2.0.0",

Then npm install.

Open the test repo in your IDE, navigate to src/playground/Playground.tsx and make sure the PlaygroundClient has correct completion, and that you can navigate to its file.

@psrpinto psrpinto self-assigned this Oct 19, 2024
@psrpinto psrpinto marked this pull request as ready for review October 19, 2024 22:05
Copy link
Collaborator

@adamziel adamziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank @psrpinto! I'm happy to get this merged as soon as the README-npm.md discussion is resolved

@psrpinto
Copy link
Member Author

README-npm.md has been removed. I also rebased with latest trunk so that this branch is up-to-date, the force-push didn't contain any other changes. From my side this is good to go :)

@psrpinto psrpinto requested review from adamziel and bgrgicak October 25, 2024 15:21
@adamziel adamziel merged commit 2ce6b10 into trunk Oct 25, 2024
9 checks passed
@adamziel adamziel deleted the remote-npm branch October 25, 2024 15:47
@adamziel
Copy link
Collaborator

Thank you so much for contributing @psrpinto!

@bgrgicak
Copy link
Collaborator

Releasing was failing in CI because the package isn't public.
I made the change in package.json and released it manually.

adamziel pushed a commit that referenced this pull request Oct 30, 2024
…npm package (#1949)

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/[email protected]
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 ===
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Type declarations broken in NPM packages
3 participants