feat: add option to public path cdn

[johanseto]

Description

The idea of this commit is to add the option to manage the publicPath of webpack to use CDN routing.
Here you can see that this is managed by webpack, the problem is that publicPath is also used in frontend platform with other purposes. So if I override publicPath the built is loading statics from the CDN in the correct behaviour but frontend platform starts with problems.

webpack recomendation

https://webpack.js.org/configuration/output/#outputpublicpath

frontend platform

https://github.com/openedx/frontend-platform/blob/master/src/initialize.js#L102

So with this change, I can affect only the web pack behavior without affecting the history web browser of the frontend platform.

Before

After

[openedx-webhooks]

Thanks for the pull request, @johanv26! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[itsjeyd]

Hi @johanv26, thank you for this contribution! Are the changes ready for review?

[johanseto]

Hi @johanv26, thank you for this contribution! Are the changes ready for review?

Yes, they are. Thanks for your attention.

[itsjeyd]

@johanv26 No problem! I'm marking this as ready for a test run.

[itsjeyd]

Hey @arbrandes, based on Ed's request for review above I just wanted to check in and see if this PR is on your radar? It doesn't seem to have made it into your queue yet.

[arbrandes]

Indeed, it wasn't on the review queue yet, in spite of having assigned myself. Adding it now!

[itsjeyd]

Great, thanks @arbrandes!

[itsjeyd]

Hey @arbrandes, any updates on when you might get around to reviewing this PR?

CC @johanv26

[arbrandes]

I don't doubt that the handling of PUBLIC_PATH in frontend-build and frontend-platform is too overloaded and thus prone to edge-case failures like the one described here, I'd first like to understand exactly that the failure being fixed is, so that we can:

1. Document how to work around it by using a separate variable like you suggest here;
2. Write unit tests that shield us from similar cases in the future.

With that in mind, do you mind giving us a step-by-step of how to reproduce the problem being fixed here?

[johanseto]

Problem

This is something that appears when you want to host MFE in buckets or use CDN to speed its loading.
In summary, the problem is when you set a publicPath with and URL with protocol instead of only a path.
eg https://d20blt6w1kfasr.cloudfront.net/learning instead /learning/
Technically webpack allows that in order to configure your generated assets targeting to another URL, like a CDN.

So the produced HTML in dist would have assets targeting a CDN or a different URL.

Reproduce

Ok, I am going to describe how to reproduce the problem.
So I didn't test it without a CDN but I thought you could test configuring in PublicPath a URL instead of /learning/
something like http://apps.{domain}:2000/learning/.

So with that change, the learning MFE would stop working.

Cause

Researching a little I found that problem is the history module because the PublicPath is used in this module as basename.
https://github.com/openedx/frontend-platform/blob/master/src/initialize.js#L102
Checking in history module in our version
https://github.com/remix-run/history/blob/v4.5.1/modules/createBrowserHistory.js#L33
I found that the basename is used with a strip-prefix.
https://github.com/remix-run/history/blob/v4.5.1/modules/createBrowserHistory.js#L52-L64
So the basename defined with a complete URL doesn't work the same in the strip, because is expected to remove the learning.
I tried to reproduce the lines of code of the history module.

Current solution

My current solution is to configure 2 variables for the "PublicPath". one for webpack and the other one for history code would use.

PUBLIC_PATH_CDN: https://d20blt6w1kfasr.cloudfront.net/learning
PUBLIC_PATH: /learning/

With this solution webpack would use PUBLIC_PATH_CDN so would create the assets targeting the CDN.
And the part of history would still be using the PUBLIC_PATH so the history module don't break.

[arbrandes]

I see the problem. But if we introduce a related but separately defined variable, then you force the operator to keep both of them synchronized, otherwise things will fail in different ways.

What would it take for us to support PUBLIC_PATH as Webpack originally intended it, instead?

[johanseto]

I see the problem. But if we introduce a related but separately defined variable, then you force the operator to keep both of them synchronized, otherwise things will fail in different ways.

What would it take for us to support PUBLIC_PATH as Webpack originally intended it, instead?

I was thinking about whether and is possible to support PUBLIC_PATH as Webpack originally intended it ...

fix it in frontend-platform.
https://github.com/openedx/frontend-platform/blob/master/src/initialize.js#L102
So with this one if a PUBLIC_PATH is configured as a URL as webpack said, the frontend platform would take only the path.
eduNEXT/frontend-platform@master...jlc/fix-capture-Public-Path

I would know your opinion if is a good way to get the path, but that is the idea.

[arbrandes]

@johanv26, yes, that last suggestion seems like a good path forward! Particularly if you can come up with some good unit tests to go with it! ;)

[johanseto]

Thanks @arbrandes, as you recommended I start the process with a PR in frontend-platform. I also add some unittests.
https://github.com/openedx/frontend-platform/pull/568/files :)

[johanseto]

Thanks @arbrandes, as you recommended I start the process with a PR in frontend-platform. I also add some unittests.
https://github.com/openedx/frontend-platform/pull/568/files :)

[itsjeyd]

@johanv26 Does openedx/frontend-platform#568 replace the changes from this PR, or complement them?

In other words, are the changes from this PR still needed or would it make sense to close this PR?

[johanseto]

@itsjeyd it makes sense to close this PR in favor of the frontend-platform PR. openedx/frontend-platform#568 has the solution, but this PR msgs conserve some context.

[openedx-webhooks]

@johanv26 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

[openedx-webhooks]

@johanv26 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.