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

feat: use vue-tsc generate vue declarations #154

Merged
merged 18 commits into from
Jan 21, 2024
Merged

Conversation

aa900031
Copy link
Contributor

Use vue-tsc to generate d.ts for Vue SFC

Reference: #22

@codecov
Copy link

codecov bot commented May 28, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (ea2ff56) 81.73% compared to head (576a293) 83.05%.

❗ Current head 576a293 differs from pull request most recent head 6d45e0b. Consider uploading reports for the commit 6d45e0b to get more accurate results

Files Patch % Lines
src/utils/vue-dts.ts 93.75% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #154      +/-   ##
==========================================
+ Coverage   81.73%   83.05%   +1.32%     
==========================================
  Files          11       12       +1     
  Lines         761      850      +89     
  Branches      120      133      +13     
==========================================
+ Hits          622      706      +84     
- Misses        137      142       +5     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aa900031
Copy link
Contributor Author

@danielroe Thank you for fixed lint, Sorry, I didn't notice that

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

I think ideally I would like to refactor this to be a separate 'vue-dts' loader. But I think we can probably merge and iterate later. wdyt @pi0?

@robinscholz
Copy link

Would be great to get this merged!

@danielroe
Copy link
Member

Thoughts @pi0?

@pi0 pi0 changed the title feat: use vue-tsc generate SFC d.ts feat: use vue-tsc generate .d.ts files for .vue sources Jul 18, 2023
@pi0 pi0 changed the title feat: use vue-tsc generate .d.ts files for .vue sources feat: use vue-tsc generate vue declarations Jul 18, 2023
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Hi again thanks for this PR and sorry for checking late. I like the general idea and the implementation but checking with current status and pnpm dev to run on fixture, it generates additional files inside src + empty ones in dist. We mighe need to check why tsHost is possibly not being passed.

Also would be nice if we have a fallback strategy to use ts loader if vue-tsc is not installed (maybe with a warn to guide to add)

@aa900031
Copy link
Contributor Author

@pi0 Thank you for review, the vfs is different after my commit 48478df, let me check the issue later, maybe need to split to single loader ?

@pi0
Copy link
Member

pi0 commented Jul 19, 2023

Standalone loader would be a nice idea!

@aa900031
Copy link
Contributor Author

Hi @pi0, based on my research, vue-tsc no longer utilizes options.host.writeFile after v1.7.8. Therefore, I have overridden ts.sys.writeFile instead.

@aa900031 aa900031 requested a review from pi0 July 22, 2023 20:50
@aa900031
Copy link
Contributor Author

@pi0 Hi again, It's been a month, could you please let me know when we can expect this to be merged?

@DevilTea
Copy link

Any update for this🥺?

@pi0
Copy link
Member

pi0 commented Nov 24, 2023

If it is important, feel free to make a new PR to rework 👍🏼 (we need to fix the correct dist output)

@aa900031
Copy link
Contributor Author

@pi0 Using the latest vue-tsc version 1.8.22 is not working. I found that rewriting ts.sys and the ts.sys used inside vue-tsc are different. I am currently looking for a way to fix this issue.

@aa900031
Copy link
Contributor Author

aa900031 commented Nov 27, 2023

Hi @pi0, Inside vue-tsc, require is used instead of import. In order to override ts.sys, it is necessary to import it in the same way as vue-tsc for them to refer to the same file, so I use same way to import typescript then override is work!

Please check that again, Thanks !

@aa900031

This comment was marked as off-topic.

@danielroe danielroe requested a review from pi0 December 26, 2023 20:34
@manniL manniL linked an issue Jan 3, 2024 that may be closed by this pull request
@aa900031
Copy link
Contributor Author

Hi everyone, I'm not sure why the review has been pause. However, since I urgently need this feature, I have independently released @aa900031/mkdist v1.5.0, which includes the functionality provided by this PR.

If you need this functionality like I do, below are a few ways to use:

Using alias in package.json:

// package.json

{
    "devDependencies": {
        "mkdist": "npm:@aa900031/mkdist@^1.5.0"
    }
}

If you use unbuild, you can use the package manager's overrides feature. Here's an example for pnpm:

// package.json

{
    "devDependencies": {
        "unbuild": "^2.0.0"
    },
    "pnpm": {
        "overrides": {
            "unbuild>mkdist": "npm:@aa900031/mkdist@^1.5.0"
        }
    }
}

@pi0 pi0 merged commit b7c396d into unjs:main Jan 21, 2024
2 checks passed
@pi0
Copy link
Member

pi0 commented Jan 21, 2024

Thanks and sorry for dalay on merge (other than review process, I'm really busy on many project so have to perorotize).

@aa900031
Copy link
Contributor Author

It's ok, I know you have many projects, Thank you so much 🙇

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.

Use vue-tsc to generate dts for Vue SFC
5 participants