Hi <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="

I think the Type compiler will look for a bundled .d.t

I already tested the solution from <a class="user-mention notranslate" data-hovercard-

Maybe it's taking the current version yup@0.32.11 whi

I have created a PR that applies fix from <a class="user-mention notranslate" data-hov

`getObject()` for Yup reports types of DefinitelyTyped,about algolia/npm-search

jablko commented on July 28, 2024 2

I think the TypeScript compiler will look for a bundled .d.ts file first, before an @types/package, so maybe this logic should be reversed:

npm-search/src/typescript/index.ts

Lines 69 to 87 in d99c31f

    
           // The 2nd most likely is definitely typed 
        
           const defTyped = isDefinitelyTyped({ name: pkg.name }); 
        
           if (defTyped) { 
        
             return { 
        
               types: { 
        
                 ts: 'definitely-typed', 
        
                 definitelyTyped: `@types/${defTyped}`, 
        
               }, 
        
             }; 
        
           } 
        
           for (const file of filelist) { 
        
             if (!file.name.endsWith('.d.ts')) { 
        
               continue; 
        
             } 
        
             datadog.increment('jsdelivr.getTSSupport.hit'); 
        
             return { types: { ts: 'included' } };

I.e. if a package both contains built-in TypeScript declarations and a @types/package exists, the built-in declarations should take precedence.

from npm-search.

Josehower commented on July 28, 2024 2

I already tested the solution from @jablko and it seems to work fine

#1089 (comment)

if @Haroenv approves I can create the PR in no time

from npm-search.

karlhorky commented on July 28, 2024 1

Oh, another problem, looks like Microsoft deleted the "create a search index" function in June:

microsoft/DefinitelyTyped-tools#456 (comment)

Maybe a new method is needed for determining this information:

npm-search/src/typescript/index.ts

Lines 24 to 40 in d99c31f

    
           export async function loadTypesIndex(): Promise<void> { 
        
             const start = Date.now(); 
        
             const { body } = await request<TypeList[]>(config.typescriptTypesIndex, { 
        
               decompress: true, 
        
               responseType: 'json', 
        
             }); 
        
             log.info(`📦  Typescript preload, found ${body.length} @types`); 
        
             // m = modules associated 
        
             // t = @types/<name> 
        
             body.forEach((type) => { 
        
               typesCache[unmangle(type.t)] = type.t; 
        
             }); 
        
             datadog.timing('typescript.loadTypesIndex', Date.now() - start); 
        
           }

from npm-search.

karlhorky commented on July 28, 2024 1

I can see two possible improvements here:

make sure to exclude deprecated definitely-typed packages (does that count as builtin or no typescript support? usually it happens when the package adds their own TS definitions, but it could also be deprecated completely)

improve the check in this project which reads the file names so it also detects yup as having typescript support (if it's correct of course) (this file exists: unpkg.com/browse/[email protected]/es/index.d.ts)

Are you interested in investigating those?

I think @Josehower would take over 2. above, I'll let him continue here.

from npm-search.

Haroenv commented on July 28, 2024 1

DM me on Twitter, I can set you up with a testing account for this :) it needs large limits

from npm-search.

karlhorky commented on July 28, 2024 1

Maybe it's taking the current version [email protected] which does not have "types": "./index.d.ts":

https://registry.npmjs.org/yup/0.32.11

from npm-search.

Josehower commented on July 28, 2024 1

I have created a PR that applies fix from @jablko

#1091

good energy 👍👍👍👍

from npm-search.

Haroenv commented on July 28, 2024 1

I think with the changed order, the point 1 no longer is needed, unless you see some cases where that would still be relevant?

from npm-search.

Haroenv commented on July 28, 2024

I think the logic for deciding what's typed is wrong in the algolia index. npm correctly detects that there's .d.ts inside the es folder, but we don't.

I can see two possible improvements here:

make sure to exclude deprecated definitely-typed packages (does that count as builtin or no typescript support? usually it happens when the package adds their own TS definitions, but it could also be deprecated completely)
improve the check in this project which reads the file names so it also detects yup as having typescript support (if it's correct of course) (this file exists: https://unpkg.com/browse/[email protected]/es/index.d.ts)

Are you interested in investigating those?

from npm-search.

Haroenv commented on July 28, 2024

Thanks for noticing that, that's indeed problematic. This probably means we need to do a reindex once this is fixed to catch packages that were added to DT since

from npm-search.

karlhorky commented on July 28, 2024

Ok added a new issue for this, since it's a bit unrelated: #1090

from npm-search.

Josehower commented on July 28, 2024

Hi @Haroenv thanks for your time,

I have been trying to setup the project on my machine so try to research a bit more about the problem or potential solutions to it, but I can't make the app to run locally is asking me a API key, i tried with the API key we use on one of our services but it didn't work.

➜  npm-search git:(master) yarn dev
Dec 6 15:22:24 🗿 npm ↔️ Algolia replication starts ⛷ 🐌 🛰 { version: '1.7.13' }
Dec 6 15:22:24 💪  Setting up Algolia OFCNCOG2CU [ 'npm-search-bootstrap', 'npm-search' ]
Dec 6 15:22:24 ⛑   API started on port 8000
Dec 6 15:22:25 {
  err: {
    name: 'ApiError',
    message: 'Method not allowed with this API key',
    status: 403,
    transporterStackTrace: [ [Object] ]
  }
}
  err: Error: Error during run
      at ./dist/src/index.js:99:23
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
Dec 6 15:22:25 Close was requested
Dec 6 15:22:25 Stopped Main gracefully

Wondering how can I get that API key to make it work of if there is an specific setup that need to be done, unfortunately I didn't have success with the setup on CONTRIBUTING.md

UPDATE: new error message after testing the setup on https://github.com/algolia/npm-search/blob/master/CONTRIBUTING.md

from npm-search.

jablko commented on July 28, 2024

That would solve the original problem in upleveled/preflight, I think: https://github.com/upleveled/preflight/blob/5af8a398ec370c79e22ab770307ec81d574c8e29/src/checks/noDependencyProblems/noDependenciesWithoutTypes.ts#L63-L67

where you're getting @types/yup for a package that contains built-in declarations.

from npm-search.

Haroenv commented on July 28, 2024

The order was chosen for performance, as DT doesn't require any extra network requests. Maybe if it's deprecated we still fall through to "included" checks, but regularly we keep DT first?

from npm-search.

jablko commented on July 28, 2024

I'm not familiar with how Algolia npm-search works, I'm kinda surprised yup falls through the first case:

npm-search/src/typescript/index.ts

Lines 64 to 67 in d99c31f

    
           // Already calculated in `formatPkg` 
        
           if (pkg.types.ts === 'included') { 
        
             return { types: pkg.types }; 
        
           }

Are GetPackage and NicePackage not the npm registry metadata, which contains "types": "./index.d.ts" for yup? https://registry.npmjs.org/yup

npm-search/src/formatPkg.ts

Lines 64 to 67 in d99c31f

    
           export function formatPkg(pkg: GetPackage): RawPkg | undefined { 
        
             const start = Date.now(); 
        
             // Be careful NicePackage modify the Object ref 
        
             const cleaned: NicePackageType | undefined = new NicePackage(pkg);

npm-search/src/formatPkg.ts

Line 134 in d99c31f

const types = getTypes(cleaned);

from npm-search.

karlhorky commented on July 28, 2024

Wonder if that's actually a bug / missing feature in the npm feature of adding types information to the registry:

npm/rfcs#126

(because in this case, the index.d.ts file for [email protected] is in the esm/ directory)

cc @orta

from npm-search.

jablko commented on July 28, 2024

Probably that version of yup was published with a version of the npm CLI/pacote that hadn't implemented the RFC yet?

from npm-search.

jablko commented on July 28, 2024

The order was chosen for performance, as DT doesn't require any extra network requests. Maybe if it's deprecated we still fall through to "included" checks, but regularly we keep DT first?

If Algolia npm-search indexes all the npm packages, and there are only about 8,000 DT types, is the performance of keeping DT first significant?

from npm-search.

karlhorky commented on July 28, 2024

@Josehower @Haroenv Thanks for creating and merging #1091 🙌

I think this hasn't taken into account the second improvement that @Haroenv mentioned above though:

make sure to exclude deprecated definitely-typed packages (does that count as builtin or no typescript support? usually it happens when the package adds their own TS definitions, but it could also be deprecated completely)

@Haroenv can you reopen this? Or should I create a new issue for this improvement?

from npm-search.

`getObject()` for Yup reports types of DefinitelyTyped about npm-search HOT 19 CLOSED

Comments (19)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	// The 2nd most likely is definitely typed
	const defTyped = isDefinitelyTyped({ name: pkg.name });
	if (defTyped) {
	return {
	types: {
	ts: 'definitely-typed',
	definitelyTyped: `@types/${defTyped}`,
	},
	};
	}

	for (const file of filelist) {
	if (!file.name.endsWith('.d.ts')) {
	continue;
	}

	datadog.increment('jsdelivr.getTSSupport.hit');

	return { types: { ts: 'included' } };

	export async function loadTypesIndex(): Promise<void> {
	const start = Date.now();
	const { body } = await request<TypeList[]>(config.typescriptTypesIndex, {
	decompress: true,
	responseType: 'json',
	});

	log.info(`📦 Typescript preload, found ${body.length} @types`);

	// m = modules associated
	// t = @types/<name>
	body.forEach((type) => {
	typesCache[unmangle(type.t)] = type.t;
	});

	datadog.timing('typescript.loadTypesIndex', Date.now() - start);
	}

	// Already calculated in `formatPkg`
	if (pkg.types.ts === 'included') {
	return { types: pkg.types };
	}

	export function formatPkg(pkg: GetPackage): RawPkg \| undefined {
	const start = Date.now();
	// Be careful NicePackage modify the Object ref
	const cleaned: NicePackageType \| undefined = new NicePackage(pkg);