Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Support custom assets prefix for webpack #1217

Closed
wants to merge 1 commit into from

Conversation

YYCoder
Copy link

@YYCoder YYCoder commented May 21, 2020

Relate to an old issue 437.

Support use custom assets prefix to serve static files like js and css, but currently only for webpack users since I'm not familiar with rollup. Still figuring how to solve this using rollup.

Personally, I think it's an important feature because there is a lot of companies use a different domain to serve their static files in order to bypass the browser request restriction of the same domain and to reduce meaningless bandwidth consumption generated by cookie.

For me, I like sapper and want to use it for our company's project, but this problem is completely preventing me from using it. I can't find any workaround to avoid it. So, I would like to add this feature, hopefully, it could help others like me.

What I do is add a placeholder(%sapper.assetsPrefix%) to get_page_hanlder.ts where generate js/css and HTTP links header, then when user run build or dev, it will read publicPath from webpack.config.js and replace it. So, the finally generated server runtime(server.mjs) could have a user-specified assets prefix. Because it needs to run build for every different publicPath, I add three subfolders in assets-prefix-webpack folder to test these three different situations. Looks weird, I know 😓.

So, for webpack users, all need to do is add a publicPath in their webpack.config.js like below.

output: {
	...config.client.output(),
	publicPath: 'https://some.domain.com/client/'
},

Note that client/ is not omitable.

@benmccann
Copy link
Member

a lot of companies use a different domain to serve their static files in order to bypass the browser request restriction of the same domain and to reduce meaningless bandwidth consumption generated by cookie

I saw an interesting article the other day that it's actually worse performance: https://blog.theodo.com/2019/09/cookieless-domain-http2-world/

@YYCoder
Copy link
Author

YYCoder commented May 22, 2020

a lot of companies use a different domain to serve their static files in order to bypass the browser request restriction of the same domain and to reduce meaningless bandwidth consumption generated by cookie

I saw an interesting article the other day that it's actually worse performance: https://blog.theodo.com/2019/09/cookieless-domain-http2-world/

Brilliant article! And thanks for your advice.

It's true that using several domains to serve assets just to save bandwidth caused by cookies might be negligible these days, so normally we limit the number of domains in case of it becomes a performance overhead. But I think, for sapper, it needs more fine-grained control for serve static files because there might be still a lot of users who need to use different domains.

@benmccann
Copy link
Member

benmccann commented May 30, 2020

Hmm, so in looking at this, it seems the configuration is being put in the webpack config and then it's being read from there in a few different places. That introduces lots of checks for .bundler throughout the code and I'm not sure that Sapper should have to be so aware of the bundler that you are using.

We are thinking about introducing a Sapper configuration file. It might be better to put any configuration in that file and then the bundlers can utilize it if necessary. I think that might be more in line with what is currently being done elsewhere

Also, I think that it would be better to allow configuring the path rather than just adding a prefix. E.g. to allow the users to serve from a path name /assets or /static rather than /client.

@YYCoder
Copy link
Author

YYCoder commented May 31, 2020

Totally agree, I think sapper configuration file is necessary, like other frameworks like next or nuxt. I' ll look into it.

@benmccann
Copy link
Member

Thanks for putting together this PR! Sapper never had a good method of handling configuration, which made it difficult to know how to integrate this feature. However, that's been addressed in SvelteKit by adding a svelte.config.cjs file . I'm going to go ahead and close this PR since we won't be adding the feature to Sapper. You might like to check out SvelteKit instead. We'll be reviewing PRs for missing features there

@benmccann benmccann closed this Mar 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants