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

build(webpack): remove publicPath so __webpack_public_path__ can be used to host assets #2835

Merged
merged 6 commits into from
Feb 10, 2021

Conversation

ad1992
Copy link
Member

@ad1992 ad1992 commented Jan 21, 2021

This will allow host to upload assets in CDN using __webpack_public_path_ as mentioned here.
For other cases host can load using script tag which will take care of loading the assets from CDN as shown here

  • Tested __webpack_public_path_ is working for non CRA apps
  • Update Readme

Blockers

  • It's not working in the CRA app, check here

EDIT
It's working fine with CRA as well I checked the same example in local. Looks like an issue with the code sandbox.

Works fine with non-CRA apps and CRA apps as well.

@vercel
Copy link

vercel bot commented Jan 21, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/luxsptd3t
✅ Preview: https://excalidraw-git-aakansha-public.excalidraw.vercel.app

@dwelle
Copy link
Member

dwelle commented Jan 27, 2021

Haven't tried locally yet. So if CRA users don't supply __webpack_public_path_ (where exactly?), they'll need to switch from /excalidraw-assets to static/js/excalidraw-assets?

@ad1992
Copy link
Member Author

ad1992 commented Jan 28, 2021

Haven't tried locally yet. So if CRA users don't supply __webpack_public_path_ (where exactly?), they'll need to switch from /excalidraw-assets to static/js/excalidraw-assets?

@dwelle so for CRA(Since CRA internally serves lazy loading assets from {hostUrl}/static/js path so the assets need to be copied to public/static/js)
But if it needs to be served from a different URL for example cdn then PUBLIC_URL needs to be updated as in env variable or CLI then the assets will be served from {PUBLIC_URL}/static/js path

@antoineduban
Copy link

This change would help us a lot at Slite! If there is anything we could do to help push this forward, let me know!

@ad1992
Copy link
Member Author

ad1992 commented Feb 10, 2021

This change would help us a lot at Slite! If there is anything we could do to help push this forward, let me know!

Yep, this change will help host apps to host their assets and works fine with CRA(downloading the example and setting public URL works) and non-CRA apps. I have been waiting for code sandbox issue to get resolved so the example doesn't break but looks like it will take some time. I am ok with merging it. @dwelle what do you say?

@dwelle
Copy link
Member

dwelle commented Feb 10, 2021

Forgot to get back to this. Feel free to merge. 👍

@ad1992 ad1992 added this to the 0.3.0 milestone Feb 10, 2021
@ad1992
Copy link
Member Author

ad1992 commented Feb 10, 2021

Will add this in next release which we will be doing soon 🎉

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.

3 participants