parser-js

parser-js copied to clipboard

This PR resolves issue #504.

• getBaseUrl() was made a separate util to make code reuse possible.
• URL validation in it is not performed because node-fetch performs its own validation at fetch time, so no repetition of this task is made. The function only ensures that url has type of string and lets node-fetch deal with the rest.
• Condition (typeof window !== 'undefined' && !options.hasOwnProperty('path')) was added for the case if getBaseUrl() gets called directly from inside of a browser (not from parseFromUrl(), but from <script>) AND, amongst all, object options doesn't already have in it its own property path with value. It is assumed in this case that getBaseUrl() was not executed yet, so it's executed. If getBaseUrl() gets called from inside of a browser AND object options HAS in it its own property path with a value - OK then, somehow it got that value, so getBaseUrl() does not get executed. This construction was inspired by test 'parse from string' in ./test/sample_browser/index.html. It is also assumed that, at the end of the day, user could have provided his own version of options.path - injected it at some point, for example.
• </> in <anonymous-schema-x> were considered custom HTML tags by browser's parser and didn't appear on screen making automated tests that assert visible elements' content to fail. So they are changed to HTML entities </> before insertion into the DOM and are replaced back to </> by browser's parser at the insertion time, thus allowing automated tests querying elements' content to pass.
• Assertion tests are made big intentionally, so they test big chunks of functionality at once, and if the test fails, one, using difftool, can quickly peek what was parsed wrong and get an idea of where to look.
• Updated puppeteer to latest version (v17.0.0 at the moment of writing) just to make it more up to date.

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

@derberg Fixed.

We are facing same issue, so curious to know when this PR could be merged and available to use?

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

@derberg are we waiting for other people to review it as well or can we merge it? 🤔

@jonaslagoni no, I was most interested with your input as you did report the related issue.

we just need to update PR description properly so GitHub closes related issues.

So, you think we can close #344 with this one too?

You right, yea, I think we can, at least that's what I am gonna do 👍

@aeworxet just added the following to automatically close the issues once released 🙂

Fixes https://github.com/asyncapi/parser-js/issues/344
Fixes https://github.com/asyncapi/parser-js/issues/504

:tada: This PR is included in version 1.16.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

@all-contributors please add @aeworxet for code

@derberg

I've put up a pull request to add @aeworxet! :tada:

:tada: This PR is included in version 2.0.0-next-major.18 :tada:

The release is available on:

• npm package (@next-major dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: