vuepress / core

Vue-Powered Static Site Generator
https://vuepress.vuejs.org
MIT License
2.16k stars 922 forks source link

feat(core): allow non-default-export client config file #1564

Closed Mister-Hope closed 1 month ago

Mister-Hope commented 1 month ago

do not require export default {} with client config file

useful for:

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 9211065904

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/core/src/app/prepare/prepareClientConfigs.ts 0 1 0.0%
<!-- Total: 0 1 0.0% -->
Totals Coverage Status
Change from base Build 9184528421: 0.0%
Covered Lines: 688
Relevant Lines: 1709

💛 - Coveralls
Mister-Hope commented 1 month ago

I'm not pretty sure about the first one, So you mean importing this file on the node side and see if the default export is available?

So what's the motivation of this? We can't just filter those files which does not have sport because we shall import them anyway to have their library and style file imported. Also, if the file is containing some style files import like css and scss, An error is expected at node side because we are not dealing them with bundlers.

And I think there is no actual performance decrease for using a namespace import, Because only the default export is expected on this file.

meteorlxy commented 1 month ago

I mean to check the file content in node side or something. As I remember, v1 did similar thing by checking export default keyword.

It could avoid the extra .map((m) => m.default).filter(Boolean) in client side.

But yes it's nice to have but not such necessary.

Mister-Hope commented 1 month ago

I think checking without ast may introduce fail positive, so I prefer the original one. I don't think ast check should be introduced here.

Mister-Hope commented 1 month ago

A e2e test about config file with no export default is added.

Mister-Hope commented 1 month ago

shit, why is windows failing (ー_ー)!!