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(cli): Add the new frontend command #1043

Merged
merged 8 commits into from Jun 21, 2023

Conversation

NoriSte
Copy link
Contributor

@NoriSte NoriSte commented May 29, 2023

This PR Introduces the new platformatic frontend CLI command that generated the frontend code to consume the Rest APIs.

New package

A dedicated package, called frontend-template is now available in the monorepo and includes

  1. The new command's code
  2. A Vite (React+TS) application that cannot work until the frontend code is generated by the new command
  3. A Playwright E2E test that tests the Vite application

Tests

The new command is tested through a single E2E test that checks that the Viter application works after the frontend code have been generated.

platformatic frontend command

platformatic frontend <url> <language>

Where:

  1. url is the URL of the Platformatic DB app
  2. language can be ts or js

The command generates two files:

  1. api.d.ts that includes all the API types
  2. api.ts or api.js that includes all the TS (or vanilla JS) functions that consume the REST APIs
    1. The TS version includes the functions with the correct types
    2. The JS version includes the JSDoc type annotations to leverage IntelliSense in JS applications

Errors

The new command reports the errors to the users for all the expected edge cases.

Docs

This PR includes the new "Generate Front-end Code to Consume Platformatic REST API" guide. Please note that the new guide should be added to the Docusaurus' sidebar here
https://github.com/platformatic/oss/blob/3fc45b08e30eff5cc0d388888bd0953186367f11/sidebars.js#L70
in order to show up in the sidebar when the docs are published.

I already opened a dedicated PR here for it.

Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
Copy link
Contributor

@marcopiraccini marcopiraccini left a comment

Choose a reason for hiding this comment

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

Great job! Tnx! Some comments, nothing blocking. But I would try to have the platformatic version of the frontend template updated "automatically" (with the script see the comment) If too complex, just update to latest platformatic, we will fix in another PR.

const { method, path } = operation

// Only dealing with success responses
const successResponses = Object.entries(responses).filter(([s]) => s.startsWith('2'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why startsWith '2'? Maybe this worth a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! Let me give some context:

  1. The hen-openapi.mjs module is a fork of the already existing client-cli one. The above lines come from there
    // Only dealing with success responses
    const successResponses = Object.entries(responses).filter(([s]) => s.startsWith('2'))
    // The following block it's impossible to happen with well-formed
    // OpenAPI.
    /* c8 ignore next 3 */
    if (successResponses.length === 0) {
    throw new Error(`Could not find a 200 level response for ${operationId}`)
    }
  2. The mentioned code has been added here by @gunjam client-cli: set response types to undefined for 204 responses, support 201 and empty objects #958
  3. In general, I preferred to keep the "fork" as much aligned as possible to the original implementation

In the code, I could link to the PR or comment with something like "Non-2xx responses lack the response body and would make the rest of the code generation throw", WDYT?

@@ -0,0 +1,66 @@
// import type { PlaywrightTestConfig } from '@playwright/test';
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should remove this comment? Or it's here because is a template? (like the dotenv one below, I guess)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I removed either of them from the various PlayWright config

"fastify": "^4.17.0"
},
"dependencies": {
"platformatic": "^0.22.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are at 0.26.0 now. Maybe we should update this like we do for all other package.json? Currently we are using this script, but this updates only the package.json of the packages.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"platformatic": "^0.22.0"
"platformatic": "workspace:*"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, thank you! I committed the change suggested by @mcollina!

@marcopiraccini
Copy link
Contributor

One more thing than can be improved @NoriSte (maybe in another PR).
I did some test, and I think we should improve the cli message when the mandatory parameters are not set, now we have:

➜ platformatic frontend                         
Something went wrong. Ensure the passed url is valid.

We should check explicitly that the mandatory parameters are present and validate them. In this case should say something like no url passed, please specify the API url and the file type (js, ts)

Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
@NoriSte
Copy link
Contributor Author

NoriSte commented Jun 12, 2023

We should check explicitly that the mandatory parameters are present and validate them. In this case should say something like no url passed, please specify the API url and the file type (js, ts)

I totally agree, thanks for pointing it out! I'll work on it as soon as I can!

Signed-off-by: Stefano Magni <nori.ste.magni@gmail.com>
@NoriSte
Copy link
Contributor Author

NoriSte commented Jun 14, 2023

One more thing than can be improved @NoriSte (maybe in another PR). I did some test, and I think we should improve the cli message when the mandatory parameters are not set, now we have:

➜ platformatic frontend                         
Something went wrong. Ensure the passed url is valid.

We should check explicitly that the mandatory parameters are present and validate them. In this case should say something like no url passed, please specify the API url and the file type (js, ts)

@marcopiraccini look at the latest commit with the solution I implemented.

Tha approach now is:

  1. Did the users pass no parameters at all?
  2. Did the users pass the language as the first parameter?
  3. Did the users pass the wrong language?

I show the main invalid-params.txt help

Url and language parameters are mandatory.

npx platformatic frontend <url> <language>

example

npx platformatic frontend http://127.0.0.1:3042 ts

where `http://127.0.0.1:3042` must be replaced with your Platformatic application endpoint and the language can be `ts` or `js`.

Refer to the [dedicated guide](https://oss.platformatic.dev/docs/guides/generate-frontend-code-to-consume-platformatic-rest-api) where the full process of generating and consuming the frontend code is described.

Then, if the users passed either the parameters but the URL is not valid I show the invalid-url.txt help

Something went wrong. Ensure the passed url is valid.

You can find more details about the available options at:
https://oss.platformatic.dev/docs/reference/cli

Then , the open-api-server-error.txt and the open-api-server-no-200 helpers did not change.

WDYT?

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.

None yet

3 participants