-
Notifications
You must be signed in to change notification settings - Fork 1.6k
LFX'25:Update the pipeline to achieve clean build without any errors #16574
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: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -142,14 +142,7 @@ netlify_install: | |
[email protected] \ | ||
[email protected] \ | ||
[email protected] \ | ||
@babel/[email protected] \ | ||
@babel/[email protected] \ | ||
@babel/[email protected] \ | ||
@babel/[email protected] | ||
@npm install --omit=dev --save-dev \ | ||
[email protected] | ||
@npm install --save \ | ||
[email protected] | ||
[email protected] | ||
|
||
netlify: netlify_install | ||
@scripts/gen_site.sh | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,36 +18,51 @@ set -ex | |
|
||
mkdir -p generated/js generated/img tmp/js | ||
|
||
|
||
tsc | ||
|
||
babel --source-maps --minified --no-comments --presets minify \ | ||
tmp/js/constants.js \ | ||
tmp/js/utils.js \ | ||
tmp/js/feedback.js \ | ||
tmp/js/kbdnav.js \ | ||
tmp/js/themes.js \ | ||
tmp/js/menu.js \ | ||
tmp/js/header.js \ | ||
tmp/js/sidebar.js \ | ||
tmp/js/tabset.js \ | ||
tmp/js/prism.js \ | ||
tmp/js/codeBlocks.js \ | ||
tmp/js/links.js \ | ||
tmp/js/resizeObserver.js \ | ||
tmp/js/scroll.js \ | ||
tmp/js/overlays.js \ | ||
tmp/js/lang.js \ | ||
tmp/js/callToAction.js \ | ||
tmp/js/events.js \ | ||
tmp/js/faq.js \ | ||
--out-file generated/js/all.min.js | ||
|
||
babel --source-maps --minified --no-comments --presets minify \ | ||
tmp/js/headerAnimation.js \ | ||
--out-file generated/js/headerAnimation.min.js | ||
|
||
babel --source-maps --minified --no-comments \ | ||
tmp/js/themes_init.js \ | ||
--out-file generated/js/themes_init.min.js | ||
#Entrypoint for esbuild to bundle and minify through an entrypoint.js file | ||
cat <<EOF > tmp/js/entrypoint.js | ||
import "./constants.js"; | ||
import "./utils.js"; | ||
import "./feedback.js"; | ||
import "./kbdnav.js"; | ||
import "./themes.js"; | ||
import "./menu.js"; | ||
import "./header.js"; | ||
import "./sidebar.js"; | ||
import "./tabset.js"; | ||
import "./prism.js"; | ||
import "./codeBlocks.js"; | ||
import "./links.js"; | ||
import "./resizeObserver.js"; | ||
import "./scroll.js"; | ||
import "./overlays.js"; | ||
import "./lang.js"; | ||
import "./callToAction.js"; | ||
import "./events.js"; | ||
import "./faq.js"; | ||
EOF | ||
|
||
# Bundle + minify with sourcemap | ||
esbuild tmp/js/entrypoint.js \ | ||
--bundle \ | ||
--minify \ | ||
--sourcemap \ | ||
--target=es6 \ | ||
--outfile=generated/js/all.min.js | ||
|
||
esbuild tmp/js/headerAnimation.js \ | ||
--minify \ | ||
--sourcemap \ | ||
--target=es6 \ | ||
--outfile=generated/js/headerAnimation.min.js | ||
Comment on lines
+55
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it still relevant that this be a separate file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it needs to be a seperate file otherwise it can break the site.I have to do a bunch more test. |
||
|
||
esbuild tmp/js/themes_init.js \ | ||
--bundle \ | ||
--minify \ | ||
--sourcemap \ | ||
--target=es6 \ | ||
--outfile=generated/js/themes_init.min.js | ||
|
||
svgstore -o generated/img/icons.svg src/icons/**/*.svg |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,6 @@ | |
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. were changes such as this required to lint, or something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes |
||
const callToActionDelayMs = 250; | ||
|
||
function handleCallToAction(): void { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,14 +11,16 @@ | |
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
import { listen , copyToClipboard , getById} from "./utils"; | ||
import { button , ariaLabel , mouseenter , mouseleave , click , active} from "./constants"; | ||
import { readLocalStorage } from "./themes_init"; | ||
Comment on lines
+14
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. review top tip: I don't know anything about Typescript but it would be worth providing a little context as to why these need to be added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to be added to make them ESModules and follow ES6 standards otherwise the .ts files will be complied as global scripts and global scripts are now legacy code which doesn't follow ES6 standard. |
||
declare const buttonCopy: string; | ||
declare const buttonDownload: string; | ||
declare const buttonPrint: string; | ||
declare const docTitle: string; | ||
declare const branchName: string; | ||
declare const Prism: any; | ||
|
||
declare var iconFile: string; | ||
let syntaxColoring = true; | ||
|
||
// All the voodoo needed to support our fancy code blocks | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,18 +11,21 @@ | |||||||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||
// See the License for the specific language governing permissions and | ||||||||||
// limitations under the License. | ||||||||||
|
||||||||||
import { listen } from "./utils"; | ||||||||||
import { click } from "./constants"; | ||||||||||
declare class Popper { | ||||||||||
constructor(a: HTMLElement, b: HTMLElement, c: any); | ||||||||||
public destroy(): void; | ||||||||||
} | ||||||||||
declare var iconFile: string; | ||||||||||
// Path to the SVG icon sprite file | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comments should be above the line?
Comment on lines
+20
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
// tracks any overlay displayed on the page (e.g. menu or popover) | ||||||||||
let overlay: HTMLElement | null = null; | ||||||||||
let popper: Popper | null = null; | ||||||||||
|
||||||||||
// show/hide the specific overlay | ||||||||||
function toggleOverlay(element: HTMLElement): void { | ||||||||||
export function toggleOverlay(element: HTMLElement): void { | ||||||||||
if (overlay === element) { | ||||||||||
closeActiveOverlay(); | ||||||||||
} else { | ||||||||||
|
@@ -35,7 +38,7 @@ function toggleOverlay(element: HTMLElement): void { | |||||||||
} | ||||||||||
|
||||||||||
// explicitly show the specific overlay | ||||||||||
function showOverlay(element: HTMLElement): void { | ||||||||||
export function showOverlay(element: HTMLElement): void { | ||||||||||
if (overlay === element) { | ||||||||||
return; | ||||||||||
} | ||||||||||
|
@@ -45,7 +48,7 @@ function showOverlay(element: HTMLElement): void { | |||||||||
} | ||||||||||
|
||||||||||
// explicitly close the active overlay | ||||||||||
function closeActiveOverlay(): void { | ||||||||||
export function closeActiveOverlay(): void { | ||||||||||
if (overlay) { | ||||||||||
overlay.classList.remove("show"); | ||||||||||
overlay = null; | ||||||||||
|
@@ -57,7 +60,7 @@ function closeActiveOverlay(): void { | |||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
function handleOverlays(): void { | ||||||||||
export function handleOverlays(): void { | ||||||||||
// Attach a popper to the given anchor | ||||||||||
function attachPopper(anchor: HTMLElement, element: HTMLElement): void { | ||||||||||
if (popper) { | ||||||||||
|
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 generate this every time and not check it in?
(This question doesn't presuppose I know the answer)
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.
esbuild needs an entrypoint for bundling the files and it does when the files are ESModules and the typescript files which is used in this repo is not ESModules but are legacy old standard type global scripts.esbuild follows the import and build a dependency graph so this temporary file is needed for esbuild for bundling the files together and minify them.
You can check the tsconfig.json the target is set to es6.
When you compile a .ts file for ex:-
tsc src/ts/faq.ts --target es6
It will throw an error because the files are not following the ES6 standard.This file is just an entrypoint and nothing else for bundling and minification.
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 not have that file exist in say src/ts/entrypoint.js rather than creating it arbitrarily?
For future maintainers, it would not be intuitive to look in gen_site.sh for a magic file.