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.

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

New build system (including 3rd party languages) #2312

Merged
merged 42 commits into from Feb 10, 2020

Conversation

joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Dec 6, 2019

The idea: Simplify the build process (that currently uses gear) to simple pure JS (with minimal dependencies) that is easy to read and understand.

Closes #2101.
Closes #2061.
Closes #2234
Closes #1425
Related #1759.
Should close #2328 also.
Closes #2334.


High level overview:

  • tools/rollup_browser.js - New build script for Browser
  • tools/rollup_cdn.js - New build script for CDN
  • alters tools/build.js to hook the new CDN and Browser builds into the old Gear system
  • tools/lib (some utils/tools that the new build scripts depend on)

This directly integrates Rollup, Terser, and CleanCSS for the minification, ES6 module support (all the languages are now ES6 modules), etc.

  • The build code for browser (before the Gear stub) is < 126 lines.
  • The build code for cdn (before the Gear stub) is < 64 lines.

The idea is clean and readable code that hopefully explains itself.

Building the browser build:

async function buildBrowser(options) {
  var languages = getLanguages()
  // filter languages for inclusion in the highlight.js bundle
  languages = filter(languages, options["languages"])

  await installDocs();
  await installDemo(languages);

  log("Preparing languages.")
  await Promise.all(
    languages.map(async (lang) => {
      await lang.compile({terser: config.terser});
      process.stdout.write(".");
     } )
  )
  log("")

  var size = await buildBrowserHighlightJS(languages)

  log("-----")
  log("Core                :", size.core ,"bytes")
  log("Languages           :", languages.map((el) => el.data.length).reduce((acc, curr) => acc + curr, 0), "bytes")
  log("Languages (min)     :", languages.map((el) => el.minified.length).reduce((acc, curr) => acc + curr, 0), "bytes")
  log("highlight.min       :", size.regular ,"bytes")
  log("highlight.min.js    :", size.minified ,"bytes")
  log("highlight.min.js.gz :", zlib.gzipSync(size.data).length ,"bytes")
  log("-----")
}

Installing demo:

async function installDemo(languages) {
  log("Writing demo files.")
  mkdir("demo")
  installDemoStyles();

  const assets = await glob("./demo/*.{js,css}")
  assets.forEach((file) => install(file))

  const css = await glob("styles/*.css", {cwd:"./src"})
  const styles = css.map((el) => (
    { "name": _.startCase(path.basename(el,".css")), "path": el }
  ) )
  renderTemplate("./demo/index.html", "./demo/index.html", { styles , languages })
}

The build output looks something like (depending on build):

Starting build.
Writing docs files.
Writing demo files.
Writing style files.
Preparing languages.
..................................
Building highlight.js min file.
-----
Core                : 34596 bytes
Core (min)          : 12521 bytes
Languages           : 131641 bytes
Languages (min)     : 74562 bytes
highlight.js        : 167053 bytes
highlight.min.js    : 87116 bytes
highlight.min.js.gz : 28811 bytes
-----
Finished build.

The most complex high level code is buildBrowserHighlightJS, but I think that's understandable since this is the code building the full browser package really.

The dependency stuff is also a bit verbose (and probably could use a simpler algorithm)... but it works and is well isolated in lib/dependencies.js, so I think it's ok for now.

@joshgoebel
Copy link
Member Author

My plan is to do this in two steps... Switch CDN/browser with one release, then finish up [NPM] with a second release... that second release is where we'd simplify build.js a bit further and remove all the old gear stuff completely.

There might also be a blocker with:
#2291

Not sure.

@joshgoebel
Copy link
Member Author

joshgoebel commented Dec 7, 2019

I do use child_process to get the git revision, which I'd love to include in the built packages. Open to thoughts on a better way to do this or if there are any concerns:

var git_sha = child_process.execSync("git rev-parse HEAD")

Perhaps there is a git-lib we could pull in to do this with just the filesystem without needing to run git externally?

The new header:

/*
  Highlight.js 9.16.2 (a40e3dda)
  License: BSD-3-Clause
  Copyright (c) 2006-2019, Ivan Sagalaev
*/

@joshgoebel joshgoebel added enhancement An enhancement or new feature package/build Issues relating to npm or packaging parser labels Dec 7, 2019
@joshgoebel joshgoebel added this to In Progress in Highlight.js Dec 7, 2019
@joshgoebel joshgoebel moved this from In Progress to Next Up in Highlight.js Dec 7, 2019
@joshgoebel joshgoebel changed the title New build system for browser and CDN WIP: New build system for browser and CDN Dec 7, 2019
@egor-rogov
Copy link
Collaborator

This will require some time to get grasp of the changes. I can't promise to do it quickly, although I'll try to find time.

Copy link
Collaborator

@egor-rogov egor-rogov left a comment

Choose a reason for hiding this comment

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

For now just some comments.
Also:

  • On export default. We should mention it in docs/language-contribution.rst. And what about language definitions in separate repos, are they also supposed to use it?
  • Is roll.js used at all? It looks like a first attempt that wasn't deleted.

tools/build.js Outdated Show resolved Hide resolved
tools/build_config.js Outdated Show resolved Hide resolved
tools/lib/bundling.js Outdated Show resolved Hide resolved
tools/lib/dependencies.js Show resolved Hide resolved
@joshgoebel
Copy link
Member Author

How does it feel overall though? Does it seem clear and easy to follow at a big picture/high level? I just wanted to make sure it's making sense before I throw more time at it.

@egor-rogov
Copy link
Collaborator

How does it feel overall though? Does it seem clear and easy to follow at a big picture/high level? I just wanted to make sure it's making sense before I throw more time at it.

At a high level it looks pretty clear and certainly makes sense. Just requires some polishing.

@joshgoebel
Copy link
Member Author

At a high level it looks pretty clear and certainly makes sense. Just requires some polishing.

Perfect. I'll circle back and get out my polish tools, and then ping you again later.

@joshgoebel joshgoebel changed the title WIP: New build system for browser and CDN WIP: New build system (including working with 3rd party languages) Dec 21, 2019
@joshgoebel
Copy link
Member Author

joshgoebel commented Dec 22, 2019

@egor-rogov You want to take another look at this now, it's a lot more polished I think.

I'm still waiting for more feedback on:

#2328

But really that only affects the actual detection of third party language packages (external_language.js)... and I've tried to keep all that bottled into a single place.

@egor-rogov
Copy link
Collaborator

@egor-rogov You want to take another look at this now, it's a lot more polished I think.

Yeah, I'm going to, but have to find a big enough time quantum...

@joshgoebel joshgoebel changed the title WIP: New build system (including working with 3rd party languages) New build system (including working with 3rd party languages) Jan 15, 2020
@joshgoebel joshgoebel changed the title New build system (including working with 3rd party languages) New build system (including 3rd party languages) Jan 15, 2020
Copy link
Collaborator

@egor-rogov egor-rogov left a comment

Choose a reason for hiding this comment

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

Generally everything looks good for me, but see some comments.

tools/lib/dependencies.js Outdated Show resolved Hide resolved
tools/lib/dependencies.js Show resolved Hide resolved
tools/lib/external_language.js Show resolved Hide resolved
tools/lib/makestuff.js Show resolved Hide resolved
tools/lib/language.js Outdated Show resolved Hide resolved
tools/build_cdn.js Show resolved Hide resolved
@joshgoebel joshgoebel force-pushed the squash_build_pipeline branch 3 times, most recently from 8169640 to 0f84777 Compare January 19, 2020 10:42
Copy link
Collaborator

@egor-rogov egor-rogov left a comment

Choose a reason for hiding this comment

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

There is one unresolved question, but it's not that big a deal. The green light to the new build system (:
(For the future, I'd prefer several smaller PRs, 'cause it's much easier to move in smaller steps...)

tools/lib/makestuff.js Show resolved Hide resolved
tools/lib/dependencies.js Show resolved Hide resolved
tools/build_cdn.js Show resolved Hide resolved
@joshgoebel
Copy link
Member Author

(For the future, I'd prefer several smaller PRs, 'cause it's much easier to move in smaller steps...)

Agree, but so much of it was interconnected and the plan came together as it happened. I see this as a pretty unusual type thing though in its complexity... should be simpler things in the future. :-)

@joshgoebel joshgoebel merged commit b64b008 into highlightjs:master Feb 10, 2020
Highlight.js automation moved this from Next Up to Done Feb 10, 2020
taufik-nurrohman pushed a commit to taufik-nurrohman/highlight.js that referenced this pull request Feb 18, 2020
* new build process for brownser/cdn/node
* detect 3rd party languages
* can run markup tests in 3rd party bundles
* can run detect tests for 3rd party bundles
* remove gear/bluebird dependencies
* add readme and license to CDN build
* CDN embeds no languages vs all if no languages are passed in the list
* change lib/highlight to lib/core for requiring the lib
* (chore)package CDN for NPM
* .min now, not .pack

Co-authored-by: Eric Prud'hommeaux <eric+github@w3.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment