-
Notifications
You must be signed in to change notification settings - Fork 2
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
add package for use with other npm packages #50
Conversation
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.
This is interesting. I could imagine it being useful, but I don't know practically what would pull it in and how. Could you provide some context on use cases for it?
this is an example of how you can use it in a project other than drupal. It is similar to what cu_bear is doing with composer but doing it in an npm way https://github.com/CU-CommunityApps/CD-cuentry/pull/1434/files require it in package.json "cwd_base": "https://github.com/CU-CommunityApps/cwd_base/tarball/main", then use it in your app.scss build @import "../../node_modules/cwd_base/sass/base.scss";
@import "../../node_modules/cwd_base/sass/cornell.scss";
@import "../../node_modules/cwd_base/sass/cwd_utilities.scss";
@import "../../node_modules/cwd_base/sass/drupal.scss"; |
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.
Seems good to me
Thank you @philwilliammee! -- and thanks @woodseowl I had the same question :) And I have another "ignorant" question: Would this package.json have any impact when npm packages are added to child themes of cwd_base -- or if we were to add npm packages to cwd_base in the future? -- if the answer is "no", you don't need to spend your time explaining, I believe you :D thanks! |
No this shouldnt have any effect on any child themes. If you were to add npm packages to cwd_base in the future you would need this file to add them. You would add them to the package.json under dependencies. It would look something like this: {
"name": "cwd_base",
"version": "1.0.0",
"description": "base theme",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "git+https://github.com/CU-CommunityApps/cwd_base.git"
},
"author": "",
"license": "ISC",
"bugs": {
"url": "https://github.com/CU-CommunityApps/cwd_base/issues"
},
"homepage": "https://github.com/CU-CommunityApps/cwd_base#readme",
"dependencies": {
"jquery": "1.12.4",
}
} |
One thing we could do after this is merged is make cwd_base a dependency of cwd_project. Then you wouldnt have the same files in both places. cwd_project would just have a build step that pulls in all of the base dependencies. |
I also wasnt sure if this PR should have gone in https://github.com/CU-CommunityApps/cwd_framework or here in cwd_base we have the same files defined in many repos what this PR attempts to do is make a central repository (source of truth) for js and css files used by many projects. So updates can be passed onto project that have them as dependencies. |
This was my concept in this issue https://github.com/CU-CommunityApps/cwd_framework/issues/2 in this issue there was talk of a |
@alisonjo315 I think this repo is the definitive source of truth for cwd theme. So if you approve this PR I'll merge it into master or if you have other ideas. |
@philwilliammee Indeed, cwd_base is definitely a Drupal theme. Thank you for all the info! I just approved, and then thought of another thing, but I think it's ok this time: Often, I put changes to cwd_base on a multidev of CD Demo, to make sure things are still ok. But in this case, it's probably not necessary. @ama39 Last call for input before Phil merges! -- we can always revise/roll-back if necessary of course :) |
@philwilliammee Now that I'm starting to get this, I agree that this belongs on CSS Framework rather than here. I think the design is that cwd_base is a Drupal theme used by Drupal sites as an adapter to the CSS Framework. So a Laravel site shouldn't be pulling in a Drupal theme. Is this just as easy to put over there? |
Updated https://github.com/CU-CommunityApps/cwd_framework/issues/2#issuecomment-952960026. And the cwd_base composer script PR, which seems to be conceptually related: #28 |
seeing if this changes my review from approved to not approved 🤞
2023-05-15: We looked at this as a full team (Monday meeting) -- there's some interest, but no one's dying to take it on, so we'll close for now, but we can always revisit it! |
This allows us to add this as an NPM dependency to a project.