Skip to content
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.

Sign up for GitHub

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

feat(icon): moved icon generation script #535

Merged
merged 8 commits into from
Dec 31, 2020
Merged

feat(icon): moved icon generation script #535

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d10b249
feat(icon): refactored package.json -> move icon generation script to…
Dec 30, 2020
87d33f4
added test
Dec 30, 2020
d335ac9
added test
Dec 30, 2020
f7337f3
added test
Dec 30, 2020
7c22a4f
removed test
Dec 30, 2020
6d73c1a
Merge branch 'master' into no_jira_icon_generation_script_moved
gullerya Dec 30, 2020
c277277
Merge branch 'master' into no_jira_icon_generation_script_moved
tveinfeld Dec 30, 2020
f83f106
changed "parallel" to "stream"
Dec 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/release-publish-npm.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
run: yarn compile

- name: Build CDN icon resolver
run: yarn build:icon-resolver --resolveStrategy=cdn --awsAccessKey=${{ secrets.AWS_VIVID_DEMO_PROD_ACCESS }} --awsAccessSecret=${{ secrets.AWS_VIVID_DEMO_PROD_SECRET }} --awsBucketName=vivid-icons-prod --awsBaseUrl=https://d3fzvwfxu7izti.cloudfront.net
run: npx lerna run "generate-icon-resolver" --parallel --scope="@vonage/vwc-icon" -- --verboseOutput --resolveStrategy=cdn --awsBucketName=vivid-icons-prod --awsBaseUrl=https://icons.resources.vonage.com --awsAccessKey=${{ secrets.AWS_VIVID_DEMO_PROD_ACCESS }} --awsAccessSecret=${{ secrets.AWS_VIVID_DEMO_PROD_SECRET }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as below, can you please use yarn to preserve convention as elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this will yield the same result? (see my comment☝️)

Copy link
Contributor

@gullerya gullerya Dec 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean yarn generate-icon-resolver, sorry, mislead you from what I've meant.


- name: Publish components (NPM)
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release-publish.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
run: yarn compile

- name: Build CDN icon resolver
run: yarn build:icon-resolver --resolveStrategy=cdn --awsAccessKey=${{ secrets.AWS_VIVID_DEMO_PROD_ACCESS }} --awsAccessSecret=${{ secrets.AWS_VIVID_DEMO_PROD_SECRET }} --awsBucketName=vivid-icons-prod --awsBaseUrl=https://icons.resources.vonage.com
run: npx lerna run "generate-icon-resolver" --parallel --scope="@vonage/vwc-icon" -- --verboseOutput --resolveStrategy=cdn --awsBucketName=vivid-icons-prod --awsBaseUrl=https://icons.resources.vonage.com --awsAccessKey=${{ secrets.AWS_VIVID_DEMO_PROD_ACCESS }} --awsAccessSecret=${{ secrets.AWS_VIVID_DEMO_PROD_SECRET }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
run: npx lerna run "generate-icon-resolver" --parallel --scope="@vonage/vwc-icon" -- --verboseOutput --resolveStrategy=cdn --awsBucketName=vivid-icons-prod --awsBaseUrl=https://icons.resources.vonage.com --awsAccessKey=${{ secrets.AWS_VIVID_DEMO_PROD_ACCESS }} --awsAccessSecret=${{ secrets.AWS_VIVID_DEMO_PROD_SECRET }}
run: yarn lerna run "generate-icon-resolver" --parallel --scope="@vonage/vwc-icon" -- --verboseOutput --resolveStrategy=cdn --awsBucketName=vivid-icons-prod --awsBaseUrl=https://icons.resources.vonage.com --awsAccessKey=${{ secrets.AWS_VIVID_DEMO_PROD_ACCESS }} --awsAccessSecret=${{ secrets.AWS_VIVID_DEMO_PROD_SECRET }}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it will yield the same result:
I think that yarn lerna will try to execute a package.json script named "lerna" (which does not exist), whereas npx lerna will run the default bin for package lerna.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't, obviously.
I'm referring to make it back as it was: yarn generate-icon-resolver just as you have it in a build script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was asked by @yinonov to remove this script from /package.json. In fact, this was the guiding motivation behind this PR.
As for myself: I don't hold a firm opinion as to where this script fits best. Therefore I suggest that @gullerya and @yinonov will discuss their opinions and reach a conclusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What makes you think that our opinions are different? Don't push us where we aren't :)
The script can be run via yarn pointing to the package folder, as it was beforehand and there is no other opinion expressed on this specific point.


- name: Publish components
run: |
Expand Down
12 changes: 7 additions & 5 deletions components/icon/build/vwc-icon-resolver-factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const
DEFAULT_ICON_FOLDER = "./build/icon",
DEFAULT_SKIP = false,
DEFAULT_RESOLVE_STRATEGY = "esm", // cdn/esm
DEFAULT_CDN_CACHE_CONTROL = "public, max-age=604800";
DEFAULT_CDN_CACHE_CONTROL = "public, max-age=604800",
DEFAULT_VERBOSE = false;

const
{
Expand All @@ -23,8 +24,9 @@ const
awsBaseUrl = DEFAULT_BASE_URL,
iconFolder = DEFAULT_ICON_FOLDER,
outputFolder = DEFAULT_OUTPUT_FOLDER,
resolveStrategy = DEFAULT_RESOLVE_STRATEGY
} = parseArgs(process.argv.slice(2), { alias: { "skip": "awsSkipUpload" } }),
resolveStrategy = DEFAULT_RESOLVE_STRATEGY,
verboseOutput = DEFAULT_VERBOSE
} = parseArgs(process.argv.slice(2), { alias: { "skip": "awsSkipUpload", "v": "verboseOutput" } }),
log = (message)=> console.log(["✓", message].join(' ')),
warn = (message)=> console.warn(["✘", message].join(' '));

Expand Down Expand Up @@ -100,7 +102,7 @@ const createDynamicResolver = function(){
const moduleName = `${path.basename(filename, path.extname(filename))}.js`;
return kefir
.fromNodeCallback((cb)=> writeFile(path.join(baseModulePath, moduleName), esModuleTemplate(content), cb))
.map(()=> ({ type: "debug", value: `"${moduleName}" module was created successfully` }));
.map(()=> ({ type: "log", value: `"${moduleName}" module was created successfully` }));
}, 5)
.beforeEnd(()=>({
type: "code",
Expand All @@ -123,7 +125,7 @@ kefir
.flatMap((code)=> kefir.fromNodeCallback((cb)=> writeFile(path.resolve(process.cwd(), outputFolder, 'icon-resolve.autogenerated.js'), code, cb)))
.map(()=> `Icon resolver module created successfully!`),
processStream
.filter(({ type })=> type === "log")
.filter(verboseOutput ? ({ type })=> type === "log" : ()=> false)
.map(({ value })=> value)
])
.onValue(log)
Expand Down
3 changes: 2 additions & 1 deletion components/icon/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
},
"scripts": {
"test": "echo \"Error: run tests from root\" && exit 1",
"build": "node ./build/vwc-icon-resolver-factory.js && tsc --allowJs --emitDeclarationOnly -d ./src/vwc-icon.js"
"generate-icon-resolver": "node ./build/vwc-icon-resolver-factory.js",
"build": "tsc --allowJs --emitDeclarationOnly -d ./src/vwc-icon.js && yarn generate-icon-resolver"
},
"bugs": {
"url": "https://github.com/Vonage/vivid/issues"
Expand Down
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
"compile": "yarn packages:build && yarn compile:styles && yarn compile:typescript",
"compile:styles": "node ./scripts/build-styles.js",
"compile:typescript": "tsc --build",
"build:icon-resolver": "cd ./components/icon && node ./build/vwc-icon-resolver-factory.js",
"git:clean": "git clean -xdf",
"watch": "node scripts/watcher.js",
"wca:json": "wca analyze components/*/src/*.?s --outFile custom-elements.json",
Expand Down