Improve testing for typescript types and bundle.js

[derberg]

Reason/Context

• Parser is not only for node.js but should work in the browser too. I recently broke bundle.js by introducing a library that doesn't work in the browser. I don't want to break it again, but I also know that when you choose a library, it is not a common habit to check if it is isomorphic (working both on server and browser). We need a way to test if the PR is not causing bundle.js to fail
• We will now generate typescript types from our JSDocs which is, in my opinion, a very good thing but also a very error-prone. We need a way to test that given PR is not breaking those types

I think testing of above can be solved with one sample project, like under one umbrella.

Description

we need some sample project in TS that will also have a ui where we can run parser and test result with
puppeteer. I was thinking that we might use asyncapi-react as it is already a TS project with UI but, but based on my experience, I know that it might look like best and easiest solution but it can backfire over time and writing a simple test project might be much more efficient.

[derberg]

Testing of the browser will be fixed with #163

[kaibra]

Hey @derberg . I am fairly new in the async-api environment and just recently had a look at all the tools available.
I have one question regarding this topic ("should also work in the browser"):
Isn't this line breaking the parser in the browser: https://github.com/asyncapi/parser-js/blob/master/lib/parser.js#L45 (the call to "process.cwd()") ?

[jonaslagoni]

Isn't this line breaking the parser in the browser: https://github.com/asyncapi/parser-js/blob/master/lib/parser.js#L45 (the call to "process.cwd()") ?

Yep good catch @kaibra. Since we aren't trying to parse a file but parsing a string here

parser-js/test/sample_browser/index.html

Line 15 in 932d2dd

const parsedFromString = await parser.parse(specFromString);

it is not triggered. Should be easily tested by parsing file instead of string in another test 😄

[derberg]

why would you try to parse a file client-side? 🤔

[kaibra]

I came to this point by looking at the react component, which has the parser as a dependency.
If I am not wrong, this line of code calls the parse function, we are talking about here:
https://github.com/asyncapi/asyncapi-react/blob/master/library/src/containers/AsyncApi/AsyncApi.tsx#L200

schema can be a string. In my case it was a string and it failed already.

I think you have a problem with your test-setup.
You are building a bundle with browserify. If you look at the docs of browserify they say:

Additionally, if you use any of these variables, they will be defined in the bundled output in a browser-appropriate way:

• process

https://www.npmjs.com/package/browserify

Looking at your bundle.js, indeed it is:

process.cwd=function(){return"/"}

[jonaslagoni]

why would you try to parse a file client-side? 🤔

Good point, don't even think it is possible without it being an url, didn't realize this yesterday 🤦

schema can be a string. In my case it was a string and it failed already.

@kaibra are you saying that you are unable to parse a string with the react component?

I think you have a problem with your test-setup.

Looked a bit further into this, and tried downloading the generated parser bundle with the browser test and opened it in Chrome where it worked fine. I tried to dig further into why the tests parses while accessing node js variables, but yea browserify to the rescue so I don't think the test-setup are faulty. But something might happen when interacting with React if you are having problems there.

[derberg]

@kaibra exactly, browserify handles process.cwd properly, so the setup of bundle generation is proper, you should basically use bundle.js in your project, it is always a part of the release, on npm and github.

I do not remember how react component handles that without using the official bundle but as you can see, the latest version is used here https://asyncapi.github.io/asyncapi-react/ and works well.

would be great if you could report a bug separately with an error message in the details.

[kaibra]

Thanks for your responses. For some reason I thought the bundle.js is just used for testing.
I had a short look again and I think I am causing the error myself....
What I tried (like maybe others before: asyncapi/asyncapi-react#75) is to integrate the asyncapi-react component in an angular application.
When debugging a small react app, using the asyncapi-react component, I found, that the shim for process coming from node-libs-browser (-> webpack) is used.
My angular app does not have process in place and when I render the component with a string-schema "ReactDOM.render(..." it breaks at process.cwd(). I now added a polyfill for process, ... to my angular app and it is working.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[aeworxet]

Following asyncapi/asyncapi-react#255, issue is postponed until React component is refactored to use the majority of types from the parser-js.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[magicmatatjahu]

@derberg Can we close this one? In Parser v2 we have improved testing of browser version https://github.com/asyncapi/parser-js/blob/next-major/test/browser/browser.spec.ts for TS types we don't need any tests - lib is written in TS so we don't have any regression between versions, only when someone will write incorrect types, but we will see that in linter stage.