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

RFC: Adding types information to the Package JSON in the registry #126

Merged
merged 6 commits into from
Feb 17, 2022

Conversation

orta
Copy link
Contributor

@orta orta commented Apr 7, 2020

What / Why

Add an unambiguous notation to the packument package.json whether a dependency includes types inside the tarball.

It would be great to know from the NPM API if a library included type definitions. This would allow the npm website, and other tooling to be able to help people make decisions about which dependencies they would like to use and support for types plays into that.

References

Prior work on the search index used by npm's site: algolia/npm-search#346

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Apr 15, 2020
@darcyclarke darcyclarke self-assigned this Apr 29, 2020
@darcyclarke darcyclarke added Needs Discussion is pending a discussion and removed Agenda will be discussed at the Open RFC call labels Apr 29, 2020
@orta
Copy link
Contributor Author

orta commented Apr 29, 2020

I can come to today's (or any) upcoming RFC meeting if you'd like me to answer questions 👍

@ruyadorno
Copy link
Contributor

@orta you're def welcome to join us! 😊 meeting is in less than half an hour though, you can find here the info on how to join: #134

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Apr 29, 2020
@orta
Copy link
Contributor Author

orta commented Apr 30, 2020

I've given this RFC an update based on our discussions yesterday (and from @Haroenv's feedback, and I've pinged the flow team for feedback )

The main focus of the update:

@orta
Copy link
Contributor Author

orta commented May 18, 2020

Hi folks - I'll come along to the next Open RFC Meeting, I'm not sure what to make of the notes from the last one 👍

@orta
Copy link
Contributor Author

orta commented May 27, 2020

OK, this RFC is updated based on the meeting today in #153

We're good to add this field to the packment during the package building phase. No-one had strong opinions around the name of the field, so I kept it as _typeRoots. RFC should be good to merge 👍

@ljharb
Copy link
Contributor

ljharb commented Jun 17, 2020

As commented in the call: it'd be great to have the paths be required to start with ./ for relative paths, disallow paths starting with / to block absolute paths, and leave open the design space for bare identifiers to be "a different package".

@orta
Copy link
Contributor Author

orta commented Jun 17, 2020

Agreed! - updated that to be explicit in the RFC ( and linking it to the TS issue on the topic microsoft/TypeScript#38249 )

@orta
Copy link
Contributor Author

orta commented Jul 15, 2020

This has a PR: npm/read-package-json#92

@ThisIsMissEm
Copy link

Looking at this, wouldn't it be better to have something like:

types: {
  typescript: "./distribution/danger.d.ts"
  flow: "distribution/danger.flow"
}

but also support specifying a package where types can be found:

types: {
  typescript: { package: "@types/danger" }
  flow: "distribution/danger.flow"
}

Which would allow for tooling to automatically install the right third-party managed types.

@orta
Copy link
Contributor Author

orta commented Sep 23, 2020

Yeah, this came up a lot during the discussions and is somewhat covered in the RFC:

This RFC would always add the relative path to an existing file if the fields are empty.

This allows for downstream consumers to be able to differentiate between a module hosting their types and a file available inside the package.

However, given there's already a very large corpus of TypeScript code with types today which will definitely include "non-relative" relative paths because "main" also supports it. I'm open to discussions for a new standard like { package: "xxyy" }, but that's kinda outside the scope for this RFC which aims to add fill the fields in if they are inferred.

I opened microsoft/TypeScript#38249 to talk about ways in which that could be described in TypeScript

@orta
Copy link
Contributor Author

orta commented Dec 16, 2020

This is now a feature in the npm site 🎉

Screen Shot 2020-12-16 at 5 59 31 PM

Which was my original goal, that said, I think there's still value in not having everyone have to replicate the TypeScript / Flow default file resolvers in their apps (with a downloaded copy of the tarball) in order know if a dep has types embedded

@wraithgar
Copy link
Member

This is going out today in v7.22.0!!

@settings settings bot removed Proposal labels Sep 21, 2021
@santialbo
Copy link

This is probably not the place to post this. Please let me know if there's a more appropiate place to put this in.
I've noticed that if a package.json doesn't explicitly specify the "types" property this makes the package to not show the DT badge on npm.com
From the TS docs

Also note that if your main declaration file is named index.d.ts and lives at the root of the package (next to index.js) you do not need to mark the types property, though it is advisable to do so.
Package: https://www.npmjs.com/package/libphonenumber-js

Screenshot 2022-02-17 at 09 48 52

index.d.ts is included

Screenshot 2022-02-17 at 09 48 24

@orta
Copy link
Contributor Author

orta commented Feb 17, 2022

I believe modern builds of npm will automatically add 'types' to your package.json (during deployment) via npm/read-package-json#103 - so you can either update your npm client to have it automatically added, or add the 'types' value yourself

@ljharb
Copy link
Contributor

ljharb commented Feb 17, 2022

I shouldn’t have to edit my package.json for that tho; npm should infer it regardless.

@SimenB
Copy link
Contributor

SimenB commented Feb 17, 2022

Is that logic impacted by "exports": {"types": "typesPath", "default": "jsPath"}? I.e. "sibling to main" also respects "sibling to resolveExports('.')" (or exports in general, I guess)?

@orta
Copy link
Contributor Author

orta commented Feb 17, 2022

I shouldn’t have to edit my package.json for that tho; npm should infer it regardless.

I am assuming that the npm search index for the site isn't reaching into the tarball fs and is instead using the packument as the source of truth for the icon (which would be replicating the work in npm/read-package-json#103 in the server ). I could be wrong though. You wouldn't have to update the package json if the deploying npm version included npm/read-package-json#103 because it gets baked into the richer package.json which is uploaded

Is that logic impacted ... exports

No idea TBH, but probably not, I think the feature in the npm site was added before esm support was unprefixed in node.

@ljharb
Copy link
Contributor

ljharb commented Feb 17, 2022

@orta i have the TS logo displayed on a ton of my packages, but i guess that's because they have DT packages instead of shipping types?

@orta
Copy link
Contributor Author

orta commented Feb 17, 2022

Because I don't have access to the npm site source code all of this discussion is just my guesswork, but I'd imagine some of those packages have a .d.ts file in the right place and it's being picked up by the read-package-json change from v7.22.0+ - I could be wrong (checking the packument file would confirm)

For example, express which doesn't ship it's types in any way still shows up with a [ DT] logo instead:
Screen Shot 2022-02-17 at 9 03 25 PM

While I'm thinking about this, both on and off-topic to this current conversation, this RFC can now either be closed or merged because the changes have been fully integrated into npm too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants