-
Notifications
You must be signed in to change notification settings - Fork 37
[WXP-4953] Launch code playground (script mode) from samples in the documentation site #143
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
base: main
Are you sure you want to change the base?
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.
I still need to understand what these 2 files do:
src/@adobe/gatsby-theme-aio/components/Code/index.js
src/@adobe/gatsby-theme-aio/components/Code/styles.css
PLAYGROUND_CLIENT_ID: ${{ secrets.PLAYGROUND_CLIENT_ID }} | ||
PLAYGROUND_CLIENT_SECRET: ${{ secrets.PLAYGROUND_CLIENT_SECRET }} | ||
PLAYGROUND_AUTH_CODE: ${{ secrets.PLAYGROUND_AUTH_CODE }} | ||
PLAYGROUND_API_KEY: ${{ secrets.PLAYGROUND_API_KEY }} |
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.
Do we really need these to be set with the secrets for the test workflow? Can't they just be empty strings?
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.
Removed
upload-playground-samples.js
Outdated
|
||
const SCAN_DIRECTORY = "./src/pages"; | ||
const MARKDOWN_EXTENSION = ".md"; | ||
const IMS_BASE_URL = "https://ims-na1-stg1.adobelogin.com"; |
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.
Why are we using the stage endpoint?
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.
Changed this. Have set these URLs to stage and prod ones in the deploy.yml
upload-playground-samples.js
Outdated
const MARKDOWN_EXTENSION = ".md"; | ||
const IMS_BASE_URL = "https://ims-na1-stg1.adobelogin.com"; | ||
const IMS_TOKEN_ENDPOINT = "/ims/token/v1"; | ||
const FFC_BASE_URL = "https://ffc-addon-stage.adobe.io/"; |
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.
Same question - why stage?
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.
Changed this. Have set these URLs to stage and prod ones in the deploy.yml
package.json
Outdated
"fix:lint": "./node_modules/.bin/markdownlint --config .github/linters/.markdownlint.yml src --fix", | ||
"lint": "docker run --rm -e RUN_LOCAL=true --env-file '.github/super-linter.env' -v \"$PWD\":/tmp/lint github/super-linter:slim-v4.10.1" | ||
"lint": "docker run --rm -e RUN_LOCAL=true --env-file '.github/super-linter.env' -v \"$PWD\":/tmp/lint github/super-linter:slim-v4.10.1", | ||
"upload-playground-samples": "node upload-playground-samples.js" |
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.
"upload-playground-samples": "node upload-playground-samples.js" | |
"_postbuild": "node upload-playground-samples.js" |
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.
Have added new build:deploy command which runs the samples
package.json
Outdated
"dev": "gatsby develop", | ||
"dev:https": "gatsby develop --https --host localhost.corp.adobe.com --port 9000", | ||
"build": "gatsby build", | ||
"build": "gatsby build && yarn upload-playground-samples", |
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.
Suggested a change below to use _postbuild
for syncing the samples. You can remove && ...
from here.
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.
Have added new build:deploy command which runs the samples
upload-playground-samples.js
Outdated
|
||
/** | ||
* Exchange an IMS authorization code for a service token. | ||
* @returns access_token. |
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.
* @returns access_token. | |
* @returns Service token. |
upload-playground-samples.js
Outdated
try { | ||
const accessToken = await getImsServiceToken(); | ||
const url = new URL( | ||
`${FFC_PLAYGROUND_ENDPOINT}/${encodeURIComponent(projectId)}`, |
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.
Why is encodeURIComponent()
required?
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.
Not needed. Removed
upload-playground-samples.js
Outdated
|
||
require('dotenv').config(); | ||
|
||
const fs = require("fs").promises; |
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.
Can we use ESM instead?
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.
Consider using fs-extra
upload-playground-samples.js
Outdated
|
||
while ((match = CODE_BLOCK_REGEX.exec(content)) !== null) { | ||
const [, language, explicitId, code] = match; | ||
const id = explicitId || Math.random().toString(36).substring(2, 8); |
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.
Shouldn't id
be mandatory? If not, with the random generated id
how will we manage updates to the code sample?
let match; | ||
CODE_BLOCK_REGEX.lastIndex = 0; | ||
|
||
while ((match = CODE_BLOCK_REGEX.exec(content)) !== null) { |
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 am assuming this does the job :)
GATSBY_FEDS_PRIVACY_ID: ${{ secrets.AIO_FEDS_PRIVACY_ID }} | ||
GATSBY_SITE_DOMAIN_URL: https://developer-stage.adobe.com | ||
NODE_OPTIONS: "--max_old_space_size=8192" | ||
IMS_BASE_URL: "https://ims-na1-stg1.adobelogin.com" |
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.
Why use stage environments?
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.
Developer docs is deployed on stage. We are planning to hand over the task of adding try tags on relevant samples to devEx team. I feel uploading samples in stage environment will help them in testing the flow.
GATSBY_SITE_DOMAIN_URL: https://developer-stage.adobe.com | ||
NODE_OPTIONS: "--max_old_space_size=8192" | ||
IMS_BASE_URL: "https://ims-na1-stg1.adobelogin.com" | ||
FFC_BASE_URL: "https://ffc-addon-stage.adobe.io/" |
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.
Use the product instance only.
const getLoader = require("prismjs/dependencies"); | ||
const components = require("prismjs/components"); | ||
|
||
const componentsToLoad = [ |
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.
Are these copied from some template? Do we need all these here?
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.
Removed unwanted components.
package.json
Outdated
"dev": "gatsby develop", | ||
"dev:https": "gatsby develop --https --host localhost.corp.adobe.com --port 9000", | ||
"build": "gatsby build", | ||
"build:deploy": "gatsby build && yarn run postbuild", |
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.
Nit: Generally when you name a script as postbuild
, it runs on its own after the build.
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 am not sure but I guess yarn doesn't run postbuild after build because the post build script is not automatically run after build.
Renamed this command now.
Description
Launch playground from samples in the documentation site
https://jira.corp.adobe.com/browse/WXP-4953
Changes include:
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: