-
Notifications
You must be signed in to change notification settings - Fork 47
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] build: Introduce SCSS support #5499
base: master
Are you sure you want to change the base?
Conversation
28a33c5
to
3b64687
Compare
package.json
Outdated
@@ -64,6 +65,7 @@ | |||
"@types/rbush": "^3.0.3", | |||
"babel-eslint": "^10.1.0", | |||
"body-parser": "^1.19.0", | |||
"bundle-scss": "^1.5.4", |
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.
only 733 weekly download
Is it well maintained ? If it's not used a lot, it might not be. Is doesn't really inspire trust
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.
Looks like https://www.npmjs.com/package/scss-bundle is more wildly used, although not updated for a long time as well
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.
yup I wasn't too sure about any of those. Essentially it just bundles with a bit of logic which has no reason to change over time - I think it's kind of a "finished" product seeing the spec. I thought about doing it by hand but there seem to be some code reorganisation taking place as well and didn't want to bother too much. We could try just piping the files back to back and see how it goes?
"dev": "npm-run-all --print-label bundle:dev --parallel server serve-static watch", | ||
"server": "node tools/server/main.cjs", | ||
"build:js": "tsc --module es6 --incremental", | ||
"bundle:iife": "rollup -c -m -- --format iife", | ||
"bundle:esm": "rollup -c -m -- --format esm", | ||
"bundle:xml": "node tools/bundle_xml/main.cjs", | ||
"bundle:dev": "npm-run-all build:js bundle:iife \"bundle:xml -- --outDir build\"", | ||
"dist": "tsc --module es6 --declaration --declarationDir dist/types && rollup -c && npm run bundle:xml -- --outDir dist", | ||
"bundle:dev": "npm-run-all build:js bundle:iife \"bundle:xml -- --outDir build\" && node tools/bundle_scss/main.cjs --out build/o_spreadsheet.css --cssOnly", |
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.
bundling should also be done when running npm run build
NOTE: before considering merging this, it will require odoo & spreadsheet-tools PR as well |
This commit adds the mecanics required to support scss files in the lib. Co-Authored-by: Lucas Lefèvre <[email protected]>
3b64687
to
55ad269
Compare
e53d73b
to
5497444
Compare
This commit adds the mecanics required to support scss files in the lib.
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist