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: OutputBundle Tagged union with 'type = chunk|asset' #3080
feat: OutputBundle Tagged union with 'type = chunk|asset' #3080
Conversation
@askbeka thank you for submitting a pull request! Thanks as well for providing the related issue number. Please do fill in the description in the PR template as well, and please be verbose. That'll really help out the folks reviewing it. |
4e74c4c
to
7b47ad0
Compare
Personally I like going for rollup/src/utils/FileEmitter.ts Line 321 in 2443783
Then when The |
@lukastaegert thanks. I will add this message. In addition to that isAsset method can be added to Plugin context, to make it easier for JS users, it can frustrating to debug issues related to spelling: isAsset(chunkOrAsset: OutputChunk | OutputAsset): chukOrAsset is OutputAsset And one question. Why types.d.ts is a declarations file why not just .ts? export const CHUNK_TYPE = 'chunk';
export interface OutputChunk {
type: typeof CHUNK_TYPE;
} Later constant could be reused. |
6fb3c67
to
ddb330c
Compare
Codecov Report
@@ Coverage Diff @@
## master #3080 +/- ##
==========================================
+ Coverage 89.06% 89.06% +<.01%
==========================================
Files 165 165
Lines 5713 5715 +2
Branches 1738 1738
==========================================
+ Hits 5088 5090 +2
Misses 383 383
Partials 242 242
Continue to review full report at Codecov.
|
Modifying this property would break Rollup anyway so I think we should be safe here.
Not sure about this. Adding new properties to the context is always a bit of maintenance overhead, needs to be documented etc. And in this case, no additional information is made available. I would prefer to avoid this in order to keep the API surface as slim as possible.
This is unfortunately necessary because this is how we create typings for the distributed JS files without publishing tons of unneeded types. Basically this file is copied unmodified into the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I think this is really getting into a good shape, just some final comments. Thanks!
68cf37c
to
799667b
Compare
799667b
to
38c3fed
Compare
@lukastaegert My apologies for the delay. Will try to check what is failing in codecov |
3f5f033
to
38c3fed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good, thanks!
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
#3079
Description
I want to make it easier to develop plugins using typescript.
It is possible since Rollup is implemented in typescript. But there are few problems.
One of which is one fixed here, and reported in the issues mentioned above