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

📝 Docs: Explain pnpm and declaration emit awkwardness #1244

Open
3 tasks done
danvk opened this issue Jan 16, 2024 · 3 comments
Open
3 tasks done

📝 Docs: Explain pnpm and declaration emit awkwardness #1244

danvk opened this issue Jan 16, 2024 · 3 comments
Labels
area: documentation Improvements or additions to docs status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request

Comments

@danvk
Copy link
Contributor

danvk commented Jan 16, 2024

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Expected

I expected to be able to define a helper module that imported vitest without error:

import { vi } from "vitest";

export const mockError = () =>
	vi.spyOn(console, "error").mockImplementation(() => undefined);

Actual

I got an extremely cryptic error:

import { vi } from "vitest";

export const mockError = () =>
//           ~~~~~~~~~
// The inferred type of 'mockError' cannot be named without a reference to 
// '.pnpm/@[email protected]/node_modules/@vitest/spy'.
// This is likely not portable. A type annotation is necessary. ts(2742)
	vi.spyOn(console, "error").mockImplementation(() => undefined);

Additional Info

Here's an example of this error happening:

The gist seems to be that declaration: true and pnpm don't work very well together. There are many, many issues about this on the TypeScript issue tracker. The original is microsoft/TypeScript#29808.

This seems to be the canonical comment discussing workarounds: microsoft/TypeScript#47663 (comment)

In my case I just turned off .d.ts emit, though this isn't ideal.

Dropping pnpm seems a little drastic, but this isn't great and create-typescript-app pushes you towards this error with its defaults. At the very least there could be something about this in the FAQ. A link to that comment in particular would have saved me some time today.

@danvk danvk added the type: bug Something isn't working :( label Jan 16, 2024
@JoshuaKGoldberg
Copy link
Owner

A link to that comment in particular would have saved me some time today.

Yeah 😬 as you said, this is really a TypeScript limitation with pnpm-style linking and I don't have any great workarounds myself. I think the right resolution would be a docs issue.

For visibility to any readers: what's happening here is TypeScript thinks it can't reasonably generate a .d.ts file that contains the mockError. This is because...

  1. mockError's type comes from the "vitest" package
  2. "vitest" is not an explicit dependency of the published package (just a devDependency)
  3. TypeScript is complaining that the published .d.ts file would need to refer to a package that may not exist in scope when referencing

In other words, TypeScript thinks mockError might be used by production consumers (it doesn't know it's test-only and not exported through the index.ts).

@JoshuaKGoldberg JoshuaKGoldberg added area: documentation Improvements or additions to docs type: feature New enhancement or request status: accepting prs Please, send a pull request to resolve this! and removed type: bug Something isn't working :( labels Jan 16, 2024
@JoshuaKGoldberg
Copy link
Owner

@all-contributors please add @danvk for ideas.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @danvk! 🎉

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

JoshuaKGoldberg pushed a commit that referenced this issue Jan 16, 2024
Adds @danvk as a contributor for ideas.

This was requested by JoshuaKGoldberg [in this
comment](#1244 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
@JoshuaKGoldberg JoshuaKGoldberg changed the title 🐛 Bug: pnpm and declaration emit are an awkward match 📝 Docs: Explain pnpm and declaration emit awkwardness Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation Improvements or additions to docs status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request
Projects
None yet
Development

No branches or pull requests

2 participants