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: Enabled resourceChain for the webjars handler #85

Merged

Conversation

jonathanmtran
Copy link

@jonathanmtran jonathanmtran commented Sep 12, 2018

Checklist
Description of change

Enabled resourceChain for the webjars handler. This enables webjars-locator to function when present (added to build.gradle overlays/resource-server/build.gradle)

webjar-locator allows us to drop the version when sourcing dependencies such as web components
https://www.webjars.org/documentation#springboot

I got the change to make from https://stackoverflow.com/questions/44200423/webjars-locator-doesnt-work-with-xml-based-spring-mvc-4-2-x-configuration

Example Use Case
<script src="https://unpkg.com/vue@2.5.16"></script>
<script type="text/javascript" src="/resource-server/webjars/uportal__content-carousel/dist/content-carousel.js"></script>

This enables webjars-locator to function when present
Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

Neat! Thanks @jonathanmtran!

@drewwills
Copy link
Contributor

This is a desirable feature.

Does it work? I had a lot of problems with it in FBMS.

I see this warning in the docs...

Following the Spring MVC instructions in a Spring Boot project will result in disabling the static content mapping configured by Spring Boot.

What does it mean? (I don't follow.)

@jonathanmtran
Copy link
Author

Does it work? I had a lot of problems with it in FBMS.

So far so good. It could benefit from additional testing

I got the change to make from https://stackoverflow.com/questions/44200423/webjars-locator-doesnt-work-with-xml-based-spring-mvc-4-2-x-configuration

@drewwills
Copy link
Contributor

👍

@drewwills
Copy link
Contributor

@jonathanmtran, @ChristianMurphy, @jgribonvald, @cousquer...

Can anyone think of a good reason not to add webjars-locator directly to resource-server-webapp? So we will always have it?

I'm inclined to do that.

@drewwills
Copy link
Contributor

FYI -- I just tested this change locally... works great! 👍

@drewwills
Copy link
Contributor

@jonathanmtran et al.,

I'm going to go ahead and bake the webjars-locator dependency into resource-server-webapp.

The benefits of having it are pretty clear, and AFAIK there aren't any downsides...

  • You can't easily convince a build system to package multiple versions of the same dependency; you'd probably have to do that by hand.
  • And even if you did that -- and actually wanted that -- you can still use the full, version specific path if you want to (AFAIK).

@ChristianMurphy
Copy link
Member

Sounds good, merge away

@cousquer
Copy link

Sounds good to me also. Great idea @drewwills !

Copy link
Contributor

@jgribonvald jgribonvald left a comment

Choose a reason for hiding this comment

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

Yes i'm for the merge too !

@ChristianMurphy ChristianMurphy merged commit 5e6f8b3 into uPortal-Project:master Feb 18, 2019
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.

5 participants