search

microsoft / code-push

A cloud service that enables Cordova and React Native developers to deploy mobile app updates directly to their users’ devices.
https://microsoft.github.io/code-push/
Other
4.34k stars 484 forks source link

fix: added ci check for declaartions and added fix for the same #827

Closed saurav0705 closed 8 months ago

saurav0705 commented 12 months ago

Issue

In the latest version for code-push web sdk no declarations files are committed due to which ts is failing.

Fix

Added outDir addition option in tsconfig-release.json which fixes the issue

image

Prevention

Added a build check so that this should not happen in future releases

image
DmitriyKirakosyan commented 8 months ago

@saurav0705 , the check passes for me even without the change (adding outDir) in this PR.

saurav0705 commented 8 months ago

@saurav0705 , the check passes for me even without the change (adding outDir) in this PR.

Hi @DmitriyKirakosyan, yes I checked this I was able to reproduce this previously and not now so might be a cache issue but can we add this check so this doesn't happen in future

DmitriyKirakosyan commented 8 months ago

@saurav0705 , if the check doesn't test the added changes, it might be more confusing than helpful. It would be great if you could modify the check to ensure it fails when outDir is absent.

saurav0705 commented 8 months ago

@saurav0705 , if the check doesn't test the added changes, it might be more confusing than helpful. It would be great if you could modify the check to ensure it fails when outDir is absent.

@DmitriyKirakosyan issue is that the package that has been published to npmjs has been generated using command npm run build and not npm run build:release due to which compiler options don't have the declarations : true due to which the .d.ts file is not generated

I think making an action that automatically publishes to npmjs so that this doesn't happen Also can you look into this https://github.com/microsoft/code-push/pull/828 as this is the latest PR and also have a command that can be run before publishing to artifactory

velimir-jankovic commented 8 months ago

@saurav0705 , if the check doesn't test the added changes, it might be more confusing than helpful. It would be great if you could modify the check to ensure it fails when outDir is absent.

@DmitriyKirakosyan issue is that the package that has been published to npmjs has been generated using command npm run build and not npm run build:release due to which compiler options don't have the declarations : true due to which the .d.ts file is not generated

I think making an action that automatically publishes to npmjs so that this doesn't happen Also can you look into this #828 as this is the latest PR and also have a command that can be run before publishing to artifactory

As already discussed, changes in this PR won't ensure release without definition files will be created. We have internal pipeline for releases, I will make necessary changes there.

Closing.