Zod- / jsVideoUrlParser

A javascript parser to extract informations like provider, channel, id, start time from YouTube, Vimeo, Dailymotion, Twitch,... urls
MIT License
234 stars 73 forks source link

TS: Error in type declaration #71

Closed romgere closed 3 years ago

romgere commented 3 years ago

Hi I have an issue when using the package with typescript :

node_modules/js-video-url-parser/lib/index.d.ts:9:15 - error TS2307: Cannot find module './provider/template' or its corresponding type declarations.

It looks like the template files are not include in the lib folder of npm package :

├── base.d.ts
├── base.js
├── index.d.ts
├── index.js
├── provider
│   ├── base-provider.d.ts
│   ├── canalplus.d.ts
│   ├── canalplus.js
│   ├── coub.d.ts
│   ├── coub.js
│   ├── dailymotion.d.ts
│   ├── dailymotion.js
│   ├── soundcloud.d.ts
│   ├── soundcloud.js
│   ├── teachertube.d.ts
│   ├── teachertube.js
│   ├── ted.d.ts
│   ├── ted.js
│   ├── tiktok.d.ts
│   ├── tiktok.js
│   ├── twitch.d.ts
│   ├── twitch.js
│   ├── vimeo.d.ts
│   ├── vimeo.js
│   ├── wistia.d.ts
│   ├── wistia.js
│   ├── youku.d.ts
│   ├── youku.js
│   ├── youtube.d.ts
│   └── youtube.js
├── testUrls.js
├── urlParser.d.ts
├── urlParser.js
└── util.js

(:point_up: node_modules/js-video-url-parser/lib)

claabs commented 3 years ago

That's my bad. I think this line just needs to be removed. There's no reason for template to be included in the release package.

saibayadon commented 3 years ago

@claabs I created a PR for that, feel free to merge it!

Zod- commented 3 years ago

I just merged and released the fix. Thanks everyone.

romgere commented 3 years ago

The issue still exists in last release 0.4.3...

It seems the PR didn't fix it :

node_modules/js-video-url-parser/lib/index.d.ts:9:15 - error TS2307: Cannot find module './provider/template' or its corresponding type declarations.

9 export * from './provider/template';
                ~~~~~~~~~~~~~~~~~~~~~

I didn't take a look to the PR before merge, but the problem does not concern this line : https://github.com/brunolazzaro/jsVideoUrlParser/blob/c10f242c8ae4299eb89d7ae5013b0efd91f5c37c/lib/provider/template.d.ts#L1

But the import of template.d.ts it self wich is not present in release package.

Following @claabs comment, do you want me to make a PR to remove the import line ?

saibayadon commented 3 years ago

@romgere Can you check by installing the package directly from the repo? yarn add https://github.com/Zod-/jsVideoUrlParser.

My PR only fixed the type problem, so if you install the package directly from the repo it will work but as you said, and following @claabs it's clear that we need to delete those files entirely since they are ignored by NPM (https://github.com/Zod-/jsVideoUrlParser/blob/master/.npmignore) thus will always break on release.

saibayadon commented 3 years ago

Just opened a PR with the line removed as indicated by @claabs. I didn't realize when I opened the PR before reading @claabs comment that the template file never gets included on the NPM package. My fix only applied if you were installing the package directly from the repo (which I guess it's a bit of an issue if installing it one way works, but releasing through NPM breaks?). I just assumed that if we fixed the type issue, there wouldn't be an issue with the file being in the exports list since it worked just fine when installing via the repo. My bad!

@Zod- Is there a way to check if this actually fixes the issue before publishing to NPM?

romgere commented 3 years ago

@romgere Can you check by installing the package directly from the repo? yarn add https://github.com/Zod-/jsVideoUrlParser

@brunolazzaro I confirm installing from GH can fixe the issue.

But a fix for npm release have to be also release I think, Thx for tackling this :pray:

rskvazh commented 3 years ago

Same problem from NPM package. Broke app build.

agalazis commented 3 years ago

any news? Can you at least reopen?

murraco commented 2 years ago

Hey @Zod-, just added a PR here to fix this issue, it would be great if you could merge it.

Thanks!

Zod- commented 2 years ago

Hope it's fixed now, I've pushed a new release.