open-ux-tools icon indicating copy to clipboard operation
open-ux-tools copied to clipboard

BUG - ui5-application-writer crashes if `loadReuseLibs` option is provided and file exists

Open IainSAP opened this issue 1 year ago • 5 comments

Description

Attempting to generate an application with the same path as an existing application is supported by @sap/generator-fiori when the --force option is passed. This works except in the case where loadReuseLibs option is turned on as this loc attempts to parse the file locate-reuse-libs.js as json. All other optional template files are json.

Steps to Reproduce

  1. Generate a ui5-application with optional locateRuseLibs on : appOptions: { loadReuseLibs: true }
  2. Generate the same app again

Note the error:

SyntaxError: Unexpected token ( in JSON at position 0

Expected results

App should be generated (overwritten) successfully or an error thrown indicating that an app exists at the specified path already.

Actual results

SyntaxError: Unexpected token ( in JSON at position 0

Screenshots

If applicable, add screenshots to help explain the problem.

Version/Components/Environment

Add any other context about the problem here OS:

  • [ ] Mac OS
  • [ ] Windows
  • [ ] Other

Root Cause Analysis

Problem

{describe the problem}

Fix

{describe the fix}

Why was it missed

{Some explanation why this issue might have been missed during normal development/testing cycle}

How can we avoid this

{if we don’t want to see this type of issues anymore what we should do to prevent}

IainSAP avatar Jan 17 '24 16:01 IainSAP

@IainSAP I think the original application should be removed by the generator before the writing starts. This would avoid reading the original file in case of JS. For JSON files it may extend a file from the original project inadvertently so better all original files are removed first IMO. Otherwise we could end up with a mix of files from each generation run especially in cases where different templates are used. I'd suggest to close this and handle it in the generator.

@tobiasqueck any opinion on this?

devinea avatar Jan 17 '24 18:01 devinea

I am with @devinea, overwriting apps is a feature that is totally independent of the writer. I would prefer that the app writers fail if there is an exiting app already, therefore, the generator needs to cleanup first and then call the writer.

tobiasqueck avatar Jan 18 '24 08:01 tobiasqueck

@devinea @tobiasqueck Thats ok we will remove the app prior to writing. However, I think we need to gracefully exit the writer and not throw this exception in the case that an existing file is found ?

I thought this writer could be used to add additional features into an existing app, or did I misunderstand?

IainSAP avatar Jan 18 '24 10:01 IainSAP

the ui5-application-writer is only intended for app generation. For adding features, we have the specific writers like the fe-fpm-writer or the odata-service-writer or the mockserver-config-writer.

How would you gracefully exit it? I would be ok with an exception stating that it requires an empty folder.

tobiasqueck avatar Jan 18 '24 10:01 tobiasqueck

the ui5-application-writer is only intended for app generation. For adding features, we have the specific writers like the fe-fpm-writer or the odata-service-writer or the mockserver-config-writer.

How would you gracefully exit it? I would be ok with an exception stating that it requires an empty folder.

Yes an exception with a clear message would be an improvement. Will add this. Thanks.

IainSAP avatar Jan 18 '24 10:01 IainSAP