-
-
Notifications
You must be signed in to change notification settings - Fork 388
Update layui w/ npm auto-update #2083
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
Conversation
|
packages/l/layui.json
Outdated
| "**/*.@(js|css|map)", | ||
| "font/*.@(eot|woff2)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "**/*.@(js|css|map)", | |
| "font/*.@(eot|woff2)" | |
| "{,components/,core/}*.js?(.map)", | |
| "css/*.css?(.map)", | |
| "font/*.@(eot|ttf|svg|woff|woff2)" |
Looking at https://www.npmjs.com/package/layui/v/3.0.0-alpha.0?activeTab=code, I think we can keep this a lot more similar to what we had before, avoiding needing to use a globstar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion.
However, I believe the proposed patterns would impose significant constraints on the dist directory structure. Layui 3.0 is still in its alpha stage, and its published directory layout may change at any time. For example, if JavaScript files are later moved into dist/js/, the pattern "{,components/,core/}*.js?(.map)" would no longer work. Given that Layui’s project structure is similar in nature to Bootstrap, I still think using a more flexible pattern such as "**/*.@(js|css|map)" is more appropriate. The dist folder does not contain unnecessary files, and all JavaScript, CSS, and source map files within it are intended for CDN distribution.
As for "font/*.@(eot|woff2)", this is mainly to maintain compatibility with the 2.x release line; supporting just eot and woff2 is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really willing to accept a globstar pattern, we avoid accepting those in configs as they can result in files being included on the CDN down the road that are not expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your concern 👍
However, Layui is a Web UI component library similar in nature to Bootstrap, which means that all JavaScript, CSS, and source map files published under the dist directory are intentional and expected assets.
If you still prefer not to accept a globstar pattern, I respect that decision. Just note that if the directory structure changes in future releases, we will need to submit another PR to update the patterns, whereas a pattern like "**/*.@(js|css|map)" would avoid repeated updates and remain valid long-term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bootstrap is not a good example here, I'd much rather that config also didn't contain globstars.
I'd be willing to accept {,*/}*.@(js|css|map), so that we're only accepting one level deep of wildcard directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! I think {,*/}*.@(js|css|map) is a perfectly acceptable compromise. Many thanks.
I’ll submit an update shortly.
Hi, I’ve updated some details in
layui.jsonto align with the Layui 3 release. (resolves #2082)Additionally, I’d like to report an issue:
Layui released versions
2.12.1and2.13.0last month, but cdnjs still lists version2.12.0:Please check why the newer versions haven’t been updated automatically. Thanks!