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(spectral): Spectral validation in the browser #6

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

starkertyp
Copy link

@starkertyp starkertyp commented Apr 20, 2023

This moves the spectral validation from a backend service to the client js bundle, hopefully speeding up the validation process.

The change will allow us to drop the second container running the spectral API as we will now only need to ship the static files to the browser, which can be done by the already bundled nginx.

Files are expected to be placed under the /spectral directory in a structure like this:

├── apic-v10.yml
├── apic.yml
└── spectral
   ├── apic-v10.yml
   ├── apic.yml
   ├── apisyouwonthate.yml
   └── functions
      ├── rule-a.js
      └── rule-b.js

The downside of this approach is a significantly larger client bundle, which is why the size checks in webpack were turned from an error to a warning.

Implements HSP-5882

This moves the spectral validation from a backend service to the client
js bundle, hopefully speeding up the validation process.

The change will allow us to drop the second container running the
spectral API as we will now only need to ship the static files to the
browser, which can be done by the already bundled nginx.

Files are expected to be placed under the /spectral directory in a structure
like this:

```
├── apic-v10.yml
├── apic.yml
└── spectral
   ├── apic-v10.yml
   ├── apic.yml
   ├── apisyouwonthate.yml
   └── functions
      ├── rule-a.js
      └── rule-b.js
```

The downside of this approach is a significantly larger client bundle,
which is why the size checks in webpack were turned from an error to a warning.

Implements HSP-5882
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 20, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0f32b49
Status: ✅  Deploy successful!
Preview URL: https://49511b98.apifant-editor.pages.dev
Branch Preview URL: https://feature-hsp-5882-spectral-in.apifant-editor.pages.dev

View logs

const end = performance.now();
console.log(`Spectral validation took ${end - start} ms`);
} catch (e) {
console.error(e);
}
Copy link

Choose a reason for hiding this comment

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

11% of developers fix this issue

opt.semgrep.generic_error_disclosure: Error messages with stack traces may expose sensitive information about the application.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

starkertyp pushed a commit that referenced this pull request Apr 25, 2023
Backport of changes introduced in #6. Not entirely sure what happens
here, this seems to be a timing issue.

Not giving an explicit `source` to the messages means they go into the
same pool as the default messages, this also means that they fall into
the same cleanup logic. The `newSpecErrBatch` function seems to mess with
that cleanup (our spectral messages clean up the "regular messages") and
the UI tries to render messages that no longer exist, crashing in the
process.

the `newThrownErrBatch` function doesn't seem to have these issues but
it requires a debounce for some reason or it won't render anything.
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.

1 participant