search

asyncapi / studio

Visually design your AsyncAPI files and event-driven architecture.
https://studio.asyncapi.com
Apache License 2.0
171 stars 98 forks source link

feat: add Cypress #1094

Closed aeworxet closed 2 months ago

aeworxet commented 6 months ago

This PR introduces for ./apps/studio-next:

Deployment to Vercel is successful and can be viewed at https://asyncapi-studio-studio-next.vercel.app

Related to https://github.com/asyncapi/studio/issues/661 Related to https://github.com/asyncapi/studio/issues/1101

changeset-bot[bot] commented 6 months ago

⚠️ No Changeset found

Latest commit: 97fc97553373a096d0925426b7ed6476b4e63612

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

netlify[bot] commented 6 months ago

Deploy Preview for modest-rosalind-098b67 ready!

Name Link
Latest commit 97fc97553373a096d0925426b7ed6476b4e63612
Latest deploy log https://app.netlify.com/sites/modest-rosalind-098b67/deploys/66ea01016254430008573954
Deploy Preview https://deploy-preview-1094--modest-rosalind-098b67.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 6 months ago

Deploy Preview for asyncapi-studio-design-system ready!

Name Link
Latest commit 97fc97553373a096d0925426b7ed6476b4e63612
Latest deploy log https://app.netlify.com/sites/asyncapi-studio-design-system/deploys/66ea01019a75210007577d87
Deploy Preview https://deploy-preview-1094--asyncapi-studio-design-system.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 6 months ago

Deploy Preview for studio-next ready!

Name Link
Latest commit 97fc97553373a096d0925426b7ed6476b4e63612
Latest deploy log https://app.netlify.com/sites/studio-next/deploys/66ea0101be0c9c00080c8465
Deploy Preview https://deploy-preview-1094--studio-next.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

aeworxet commented 6 months ago

@Amzani @KhudaDad414 @asyncapi/bounty_team

aeworxet commented 6 months ago

I started with the addition of Cypress so I could spot regressions during migration to CodeMirror. Comparison with PR #1062

@Amzani A couple of questions:

Amzani commented 6 months ago

@aeworxet

Is there a design file (Figma maybe?) where I can take unique names for all HTML elements from, so testing frameworks could find them in the webpage (same as I gave the name data-testid="button-settings" to the Settings button?)

Not for the existing studio, we only have one for the new improvements that will be integrated once we finish the NextJS migration: https://www.figma.com/proto/bPMB3lkMTOOMuKk0oGLuNm/Studio?type=design&node-id=96-2392&scaling=contain&page-id=0%3A1&starting-point-node-id=2%3A2

What names should I give to HTML elements' data-* attributes: data-testid, data-test, or data-cy?

I vote for data-test for simplicity and in case we want to switch to an other testing tool in the future.

Should there be a separate spec file for components' testing, or, in the case of Studio, only full flow (e2e) makes sense?

The goal is regression test, so I would suggest full flow (e2e), we can encourage components testing for new feature we add to the project.

@KhudaDad414 WDY?

Amzani commented 6 months ago

I would remove CodeMirror Integration from the scope of this issue for now, until we merge everything.

aeworxet commented 6 months ago

I'm working on code generation for Studio, and after POST request to https://api.asyncapi.com/v1/generate I get

Version 3.0.0 is not supported.
Please use latest version of the specification.

Code, responsible for this is https://github.com/asyncapi/parser-js/blob/master/src/ruleset/functions/isAsyncAPIDocument.ts#L25 https://github.com/asyncapi/parser-js/blob/master/src/constants.ts#L22 and the file which is searched for for 3.0.0 HAS it in master: https://github.com/asyncapi/spec-json-schemas/blob/master/index.d.ts#L12

It needs to be checked on the server what version of ./node_modules/@asyncapi/specs/index.d.ts it contains. @smoya

Amzani commented 6 months ago

@aeworxet we should remove the dependency with https://github.com/asyncapi/server-api in the NextJS version of studio as this repository might be deprecated soon.

So the idea is to directly use the generator library.

Not all of them supports V3, AFAIK the following supports V3

If < V3 all the generators should work as expected.

smoya commented 6 months ago

So the idea is to directly use the generator library.

@Amzani would you mind linking to where that decission has been made? Just to be in sync with https://github.com/asyncapi/server-api/issues/576 where the idea, afaik, was to keep a server but the code living in CLI repo.

Amzani commented 6 months ago

@smoya I was assuming that if only Studio uses this API there is no need to maintain it anymore thus deprecating the server-api repo and not migrate it to CLI to reduce complexity.

aeworxet commented 5 months ago

So the idea is to directly use the generator library.

@Amzani

@KhudaDad414 suggests the opposite, to stick with server-api. So which one should it be?

Should I invest time into rewriting the logic for SSR, or should I just bugfix the current approach?

(https://github.com/asyncapi/studio/pull/1094#issuecomment-2118721184 is still relevant then)

KhudaDad414 commented 5 months ago

@aeworxet IMHO it makes sense to bug fix the current code base. By that I mean sticking to the server-api approach.

As Fran said in the original issue, migration is already hard enough, trying to implement new features at the same time would quadruple the complexity.

aeworxet commented 5 months ago

@KhudaDad414 'Code Generation' functionality still requires checking of the ./node_modules/@asyncapi/specs/index.d.ts version on api.asyncapi.com as per https://github.com/asyncapi/studio/pull/1094#issuecomment-2118721184 since it is the server that is returning the error.

image

KhudaDad414 commented 5 months ago

@aeworxet Since you get the error on the current Studio as well, I would consider resolving it to be out of scope for this Bounty Issue.

We can have another issue and discuss it over there.

aeworxet commented 5 months ago

I'm investigating options for deployment of Studio to a Docker container, and the possibility that the server part might become completely independent, up to being hosted fully in a CLI with invocation through asyncapi server start, led me to a question if deployment of Studio to a Docker container as a full Next.js app (with API Routes, SSR, and others) is needed at all.

The most runtime-dynamic part of Studio currently is the generation of templates, and it can be done through a POST request to an API from a static website (it is made this way currently, in fact.)

I'm not sure OG preview generation is a mandatory functionality for a containerized Studio, it can be left reserved for https://studio.asyncapi.com

I have deployed to https://asyncapi-studio-studio-next.vercel.app a build of Studio with output: 'export' (explanation what 'export' build is, features that are NOT supported.)

The whole 'export' build is nine (9) Mb, it has First Contentful Paint 0.4 s (+ better overall performance score in Lighthouse,) and it doesn't seem to lack any functionality.

Should this version of Studio's build be used for a Docker container?

Amzani commented 5 months ago

The most runtime-dynamic part of Studio currently is the generation of templates, and it can be done through a POST request to an API from a static website (it is made this way currently, in fact.)

We want to enable long term features like authentication for instance. generation of templates using API to POST should be something we could migrate IMO as we don't want to maintain an entire repository just for 1 single POST /generate method instead of using directly the generator library.

Should this version of Studio's build be used for a Docker container?

How complex is having everything in Docker image?

aeworxet commented 5 months ago

@Amzani

How complex is having everything in Docker image?

It is supposed to be straightforward but I ran into an error on a command that executes fine on desktop, so I'm investigating it.

image

aeworxet commented 5 months ago

I hoped to have for ouput: 'export' a 20 MB Docker image (5 MB alpine linux + 9 MB static Studio + several MB for nginx,) but for output: 'standalone' Node.js v18 alone takes 125 MB, and overall Docker image size is 282 MB (compiled Studio is 20 MB, though.) The previous Docker image was 91 MB.

A request to the API works as a POC.

image

I attempted to delete something unneeded, but it seems that everything in this Docker image is used for one process or another:

image

Try image:

cd ./apps/studio-next
docker build .
docker images # the freshly built Docker image is most probably the topmost one
docker run -p 3001:3001 [image_name]

http://localhost:3001
sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

aeworxet commented 4 months ago

@KhudaDad414 How should I handle these ESLint's issues? They are not mine, but should I rewrite them anyway, disable ESLint rules, or should these simply be ignored due to future rewrite?

./src/components/Navigation.tsx
88:17  Warning: Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.  sonarjs/cognitive-complexity

./src/components/Navigationv3.tsx
159:17  Warning: Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.  sonarjs/cognitive-complexity

./src/components/common/Switch.tsx
14:27  Error: 'e' is defined but never used.  @typescript-eslint/no-unused-vars
KhudaDad414 commented 4 months ago

@aeworxet they should be ignored. we are already ignoring them in sonar. https://github.com/asyncapi/studio/blob/master/.sonarcloud.properties

aeworxet commented 4 months ago

Disabled ESLint's rules on warnings and the error.

aeworxet commented 4 months ago

SonarCloud is frozen.

image

Sergio was dealing with this issue several days ago https://github.com/asyncapi/parser-js/pull/1008#issuecomment-2208211553

aeworxet commented 3 months ago

Can this PR be merged, or is there anything else that needs to be done on it?

Amzani commented 2 months ago

@aeworxet could you fix the conflict ? Thanks

aeworxet commented 2 months ago

@Amzani Fixed.

aeworxet commented 2 months ago

I had to rename

ConvertVersion -> AsyncAPIConvertVersion

which was renamed recently in @asyncapi/converter v1.5.0.

In the change's PR only the name was changed, so all functionality is supposed to remain the same, but pinging @jonaslagoni just in case.

aeworxet commented 2 months ago

@KhudaDad414 Can you please dismiss your change request since I have removed all Docker-related code?

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

KhudaDad414 commented 2 months ago

/rtm