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

Added support multiple documents for Scalar #2069

Merged
merged 1 commit into from
Apr 4, 2025

Conversation

altro3
Copy link
Collaborator

@altro3 altro3 commented Apr 2, 2025

Scalar 1.28.11
Swagger UI 5.20.3

@altro3 altro3 changed the title Add support multiple documents for scalar Added support multiple documents for Scalar Apr 2, 2025
@altro3 altro3 requested review from graemerocher and sdelamo April 2, 2025 08:15
@altro3 altro3 force-pushed the scalar-multiple branch 2 times, most recently from 0d9aa51 to 7c97eaf Compare April 2, 2025 11:20
@altro3 altro3 added type: improvement A minor improvement to an existing feature type: enhancement New feature or request and removed type: improvement A minor improvement to an existing feature labels Apr 2, 2025
@graemerocher
Copy link
Contributor

is there a way we can pull the scalar JS files from CDN? Having huge JS files into this repo makes it harder to review changes

@altro3
Copy link
Collaborator Author

altro3 commented Apr 3, 2025

@graemerocher That's what we do. We just run the bash script and it downloads all the js files from the CDN - I don't download anything manually.

Look at this file: https://github.com/micronaut-projects/micronaut-openapi/blob/6.15.x/download-js-files.sh

I run it to update the UI scripts

@graemerocher
Copy link
Contributor

ok but why isn't that file part of the build and why are we checking into source control these downloaded files?

@altro3 altro3 force-pushed the scalar-multiple branch from 7c97eaf to 5a53b04 Compare April 3, 2025 09:12
@altro3
Copy link
Collaborator Author

altro3 commented Apr 3, 2025

I think it's historically been like that. Scripts used to be loaded from external sources, and a couple of years ago users really asked to localize scripts to avoid external requests - I did it, but the build process didn't change. If you know how to improve this process and not store files in git - that's great, but I don't know how to do it

@graemerocher
Copy link
Contributor

@melix do you know if we can do this in the Gradle build? Makes me uncomfortable from a security perspective maintaining this javascript code in this repository.

@melix
Copy link
Contributor

melix commented Apr 3, 2025

That should definitely be done as part of the build, there's no reason not to do it.

@altro3
Copy link
Collaborator Author

altro3 commented Apr 3, 2025

In fact, I don't quite understand how you propose to integrate this into the build process. Currently, the files are stored in Git, but this eliminates unexpected problems. That is, if you start loading these files with each build, then you will either have to strictly tie to the version of each of the scripts, or the latest version of the script will always be loaded, which can sometimes lead to problems. Unlikely, but it can.

What I mean is that the current solution, although not the most beautiful, there are definitely no problems with it because the script versions are updated manually, not automatically.

@graemerocher
Copy link
Contributor

they should be versioned like any other artefact. In fact if we can get vulnerability checkers to trigger for them even better.

@melix when you get a minute can you look into this?

@melix melix self-assigned this Apr 3, 2025
@melix
Copy link
Contributor

melix commented Apr 3, 2025

Yes, these dependencies should be fetched at build time like any other dependency. The difference is that we're in JS world, so a fixed version is not... fixed in time!

@melix
Copy link
Contributor

melix commented Apr 4, 2025

So I think we can merge this PR independently of the rework of how these libraries are fetched from CDN. I will work on a separate PR clean that up.

@altro3
Copy link
Collaborator Author

altro3 commented Apr 4, 2025

agree

Scalar 1.28.11
Swagger UI 5.20.3
@melix melix merged commit b5e81d6 into micronaut-projects:6.15.x Apr 4, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants