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(dts-generator): add download-apijson API and CLI #454

Merged
merged 3 commits into from
May 7, 2024
Merged

Conversation

akudev
Copy link
Contributor

@akudev akudev commented May 3, 2024

No description provided.

@akudev akudev force-pushed the download-apijson branch from 3f3d4cb to 2951f2d Compare May 6, 2024 12:10
@akudev akudev requested review from petermuessig and codeworrior May 6, 2024 12:10
@akudev akudev force-pushed the download-apijson branch from 2951f2d to 2a44b90 Compare May 6, 2024 12:22
@akudev akudev force-pushed the download-apijson branch from 2a44b90 to b76b5ca Compare May 6, 2024 12:33
@akudev
Copy link
Contributor Author

akudev commented May 6, 2024

This change consists of these major areas:

  1. Moving the implementation in ui5-metadata.js from the test to dts-generator and converting it to an ES module (plus moving dependencies)
    • the directory preparation has been written anew to prevent accidental deletion of user directory content, which can happen with the old approach which was meant for hardcoded safe paths
  2. Changing the usage of the utils in the test to the moved ones
  3. Creating a new CLI in download-apijson.ts, which is similar like the still existing download script in the test package, but with reduced scope, as no directives are needed (the UI5 libraries do not need to be built, we can get api.json and resulting d.ts files directly)
  4. Defining the CLI arguments and docu in arguments-download-apijson.ts
  5. Lots of documentation extension and adaptation for the new API/CLI
  6. Renaming the downloaded api.json files (also renames the test content) for clearer file names
    • sap.ui.core.designtime.api.json -> sap.ui.core.api.json
  7. Declaring another binary and explicitly naming the existing one - twice

Regarding the binary:
Formerly it was not named, so the package name was supposedly the binary name(??). But in fact, the "@UI5" prefix was ignored when the symlink was created. So unintentionally, the binary is simply "dts-generator". Hence the new main binary is named explicitly "ui5-dts-generator" and the new util: "ui5-download-apijson". The old "@ui5/dts-generator" is added for compatibility. Not 100% sure about that.

"bin": {
    "@ui5/dts-generator": "dist/index.js",
    "ui5-dts-generator": "dist/index.js",
    "ui5-download-apijson": "dist/download-apijson.js"
  },

Copy link
Contributor

@petermuessig petermuessig left a comment

Choose a reason for hiding this comment

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

Just some small comments - overall looks ok

@akudev
Copy link
Contributor Author

akudev commented May 7, 2024

@petermuessig
Now I remember why I wrote the imports like I did:

      
import { writeFile } from "fs-extra";
               ^^^^^^^^^
      SyntaxError: Named export 'writeFile' not found. The requested module 'fs-extra' is a CommonJS module, which may not support all module.exports as named exports.
      CommonJS modules can always be imported via the default export, for example using:
      
      import pkg from 'fs-extra';
      const { writeFile } = pkg;

Copy link
Member

@codeworrior codeworrior left a comment

Choose a reason for hiding this comment

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

LGTM.

@akudev
Copy link
Contributor Author

akudev commented May 7, 2024

I just re-checked: even though "@ui5/dts-generator": "dist/index.js", in the binaries section of package.json gives a warning and the binary (symlink in .bin) is not named like this, only with this entry, users can continue to write npx @ui5/dts-generator, hence I leave it in.

@akudev akudev merged commit 49f0db2 into main May 7, 2024
7 checks passed
@akudev akudev deleted the download-apijson branch May 7, 2024 07:27
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.

3 participants