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

Pre-compiled files? #678

Closed
idleberg opened this issue Dec 15, 2014 · 43 comments
Closed

Pre-compiled files? #678

idleberg opened this issue Dec 15, 2014 · 43 comments
Assignees
Labels
package/build Issues relating to npm or packaging

Comments

@idleberg
Copy link
Contributor

I guess I'm not the only one looking for a way to get a pre-compiled files of hljs. So far, I tried several ways to overcome this, but none of the following was completely satisfying.

Bower

Bower is probably where pre-compiled files would fit best, but the package available is not official and outdated (as you are aware).

npm

The package distributed on npm is made for — and that's fine, I guess, since this is what npm is primarily made for. As I understand it I cannot use the node package to embed it on a website. So, instead I tried to build it using Gulp. Since it's possible to specify any GitHub repository (if it comes with a package.json) as a dependency, I tried building hljs using node.

"devDependencies": {
  "highlight.js":   "isagalaev/highlight.js"
},
 "scripts": {
  "postinstall": "node node_modules/highlight.js/tools/build.js :common"
}

Unfortunately, this does not seem to work due to a problem with file permissions (I wonder if including an empty build folder would solve this). Next I tried gulp-shell to build directly from gulp, but the tasks aborts due to missing dependencies (of hljs).

cdnjs

Ultimately, I ended up using a Gulp task that downloads JS and CSS from cdnjs. It works, but it left me with a somewhat awkward feeling.

Is their another possibility I'm overlooking? Are there any plans to make this easier in the future? I suspect many users would welcome the idea to get pre-compiled files and Bower is maybe the right place for it. Just my two cents.

@idleberg
Copy link
Contributor Author

PS: I just found out about another unofficial package on Bower. For now that will do the trick for me, but the question is the maintainer's future commitment.

@isagalaev
Copy link
Member

Since recently we maintain a repo of pre-compiled files that is the official source for CDNs: https://github.com/highlightjs/cdn-release. It's updated with each release automatically. Will that work for you?

@idleberg
Copy link
Contributor Author

Hi Ivan, that looks promising. Is there anything speaking against the addition of a package.json to that repo, so one could download it with npm as well? Afaik, a barebon private package would already do, e.g.

{
  "name": "highlight.js",
  "version": "8.4.0",
  "description": "a syntax highlighter written in JavaScript",
  "private": true,
  "license": "BSD"
}

@grabus
Copy link
Contributor

grabus commented Dec 15, 2014

you can try something like that:

"devDependencies": {
  "highlight.js":   "isagalaev/highlight.js"
},
 "scripts": {
  "postinstall": "cd ./node_modules/highlight.js && npm i && node ./tools/build.js :common"
}

@idleberg
Copy link
Contributor Author

@grabus Did you try this? I'm getting a issue with permission

npm WARN cannot run in wd

@grabus
Copy link
Contributor

grabus commented Dec 15, 2014

@idleberg Yes, it works for me. But I use it with grunt like this:

  grunt.registerTask 'highlight-build', 'highlight-build', ->
    exec = require('child_process').exec
    cb = @async()
    exec 'cd ./bower_components/highlight && npm i && node ./tools/build.js coffeescript css javascript json ruby xml', {}, (err, stdout, stderr) ->
      cb()

  grunt.registerTask 'build-libs', [
    # ...
    'lodash:build'
    'highlight-build'
    # ...
  ]

You can read here about your issue, there are a couple of solutions.

@Sannis Sannis added the on hold label Jan 10, 2015
@isagalaev
Copy link
Member

Getting back to this…

@idleberg yes, I think there's no harm in including a package.json in CDN builds. Could you make a pull request adding that to the build tool? The CDN-related build module is in tools/cdn.js, and may be you could reuse the package.json that we build for npm: https://github.com/isagalaev/highlight.js/blob/master/tools/node.js#L115

@isagalaev isagalaev removed the on hold label Feb 3, 2015
@Sannis Sannis added the code label May 20, 2015
@Sannis Sannis added this to the 8.7 milestone May 20, 2015
@Sannis Sannis self-assigned this May 20, 2015
@kilianc
Copy link

kilianc commented May 30, 2015

gulp.task('hjs', function (done) {
  var opts = {
    cwd: __dirname + '/node_modules/highlight.js'
  }

  var npmInstall = spawn('npm', ['install'], opts)
  npmInstall.stdout.pipe(process.stdout)
  npmInstall.stderr.pipe(process.stderr)

  npmInstall.on('close', function (code) {
    if (0 !== code) throw new Error('npm install exited with ' + code)

    var build = spawn('node', ['tools/build.js', '-n', 'json'], opts)
    build.stdout.pipe(process.stdout)
    build.stderr.pipe(process.stderr)

    build.on('close', function (code) {
      if (0 !== code) throw new Error('node tools/build.js exited with ' + code)
      done()
    })
  })
})

@idleberg this is what I came up with for https://github.com/kilianc/rtail and it works

@idleberg
Copy link
Contributor Author

idleberg commented Jul 8, 2015

@kilianc Sorry for the late reply. Unfortunately, this does not work for me. What's spawn in your code? (I tried gulp-spawn)

@kilianc
Copy link

kilianc commented Jul 8, 2015

@idleberg require('child_process').spawn look at https://github.com/kilianc/rtail/blob/develop/gulpfile.js#L69

@idleberg
Copy link
Contributor Author

idleberg commented Jul 9, 2015

@kilianc This is great, thanks a lot!

@Sannis Sannis removed their assignment Jul 14, 2015
@Sannis Sannis removed this from the 8.7 milestone Jul 24, 2015
@Sannis Sannis added the on hold label Jul 24, 2015
@Sannis
Copy link
Collaborator

Sannis commented Jul 27, 2015

@idleberg, is @kilianc snippet solved your problem?

@idleberg
Copy link
Contributor Author

Yes, thank you! You should think about using that yourself or at least reference it in the Wiki

@Sannis
Copy link
Collaborator

Sannis commented Jul 27, 2015

@isagalaev, what are you think about separate page in docs to reference all modules/packaging issues?

@Sannis Sannis added this to the 8.8 milestone Jul 27, 2015
@isagalaev isagalaev removed this from the 8.8 milestone Jul 27, 2015
@isagalaev
Copy link
Member

I've lost track of this thread, to be honest, but I promise to get back to it as soon as I deal with new-styles in #348.

@joshgoebel joshgoebel added this to the 9.18 milestone Dec 6, 2019
@noraj
Copy link
Contributor

noraj commented Dec 11, 2019

@isagalaev
I'm using a static website generator, I'm using Gulp to copy the assets from node_modules, build stuff etc. But

  1. I don't want to use an external version of the JS (ex: CDN)
  2. I can't build it from npm package since the build tool is not shipped in it

Of course highlight.pack.js should not be included in the npm package for the reasons mentioned in previous comments but providing the build tool will allow anyone to build it (the pack file) in Gulp or Grunt tasks and automate the process like for all other npm package. Having to clone the source repo to build it make the automating process unnecessary more difficult. Not only more difficult to install/build but also having "highlight.js": "git+https://github.com/highlightjs/highlight.js.git", in your package.json it will be harder to track the version of hljs used. It is still possible to use tags to install a specific version from git but automated dependency checker like depfu, renovabote, dependabot, etc. won't work anymore and version update/check will have to be done manually :(

So no providing the build tool in the npm package is forcing to have a bad dependency version management because of the use of git instead of the npm registry.

Demo workaround

package.json

{
  "name": "XXX",
  "version": "0.0.1",
  "private": true,
  "devDependencies": {
    "gulp": "^4.0.2",
    "highlight.js": "git+https://github.com/highlightjs/highlight.js.git#9.16.2"
  }
}

gulpfile.js

// Load plugins
const { series, parallel, src, dest, task } = require('gulp');
const { exec } = require('child_process');

task('build',
  parallel(
    hljs_style,
    series(hljs_build, hljs_webpack)
));
task('build').description = 'Build the static website';
task('default', series('build'));
task('default').description = 'build';

// Copy Highlight.js styles
function hljs_style() {
  return src('node_modules/highlight.js/src/styles/*.css')
    .pipe(dest('source/css/highlight.js/'));
};

// Build Highlight.js webpack
function hljs_build() {
  exec('npm i', {cwd: 'node_modules/highlight.js'});
  return exec('node tools/build.js', {cwd: 'node_modules/highlight.js'});
};

// Copy Highlight.js webpack script
function hljs_webpack() {
  return src('node_modules/highlight.js/build/highlight.pack.js')
    .pipe(dest('source/js/'));
};

@joshgoebel
Copy link
Member

Related:

You might be right, but I'm starting to think the opposite solution is correct. Both the JS distributable and the NPM are "built from source", so we are currently only distributing distributabled assets, with the original source code being on GitHub.

  • The CDN distributes the "cooked" web-usage library
  • The NPM distributes the "cooked" node library

This makes sense to me.

The issue seems to be that a lot of people use npm for versioning static assets... so it seems we really need:

  • The CDN distributes the "cooked" web-usage library
  • The NPM distributes the "cooked" node-usage library
  • NPM also distributes the "cooked" web-usage library
  • The source remains on GitHub.

That seems to answer what MOST people are asking for I think. With version 10 I was planning on throwing in a pre-built minified "common" build and see if that helps.

Nothing will likely change here until post 10.0 anyways (I'd like the next release to be 10.0) since any changes to the build process are most likely going to be built on top of the new build system, not the old one.

@joshgoebel
Copy link
Member

joshgoebel commented Dec 11, 2019

I see some big advantages to keeping NPM as a built deliverable rather than raw source files (which might be a possibility in the future). People using the raw GitHub repo to build with should be uncommon (correct me if you think I'm wrong).

Having it as a built asset allows us to make larger behind the scenes changes to how the library is written and structured while still providing the same old "npm API" (same file path locations, mostly same deliverable structure, etc). This ensures stability for those using npm normally, and also allows us to iterate faster with the core library.

I'm not sure we want to encourage a lot of people to use the raw source if they don't truly need to... in my experience what people do then is hook into it in custom ways that make their project and ours project more fragile (for new releases). I'm not speaking ill of anyone here (and indeed this is all OSS, so awesome), just pointing out that this often results in more fragility and maintenance time on both sides, increases the likelihood of bugs, etc...

We can only really guarantee our external documented API behavior, not the internal code/libs/etc and it's behavior...

Also, often many of the things I see being done could be done better with a standardized plugin API (which we're pursuing).

Those are my thoughts.


IF we shipped a cooked "packed" "binary" with the NPM... what do you see as the big disadvantage to that vs including TWO full copies of the source, cooked and raw...?

@joshgoebel
Copy link
Member

@noraj Is there some reason why the static CDN build doesn't work for you:

https://github.com/highlightjs/cdn-release

Fetch it once, done.

@noraj
Copy link
Contributor

noraj commented Dec 14, 2019

Because it's for a project that need to work offline and that need to be self-contained, so I need all the assets locally.

@joshgoebel
Copy link
Member

Because it's for a project that need to work offline and that need to be self-contained, so I need all the assets locally.

Why couldn't/wouldn't you just vendor it into a directory of your choice (from the CDN url or from the CDN repo) and then only download the file if it was missing? So after the very first run to build your project would work just fine entirely offline.

@joshgoebel
Copy link
Member

joshgoebel commented Dec 14, 2019

I think what some people want is for cdn-release to just be packaged up into a NPM package called highlight-js-assets or something...

  • If you want to use Highlight.js server-side, use highlight-js package
  • If you want to use Highlight.js assets client-side, use highlight-js-assets package

Thoughts? That kind of makes sense to me. What I'm less sure about is merging both into a single large repo.

@marcoscaceres Do you have any thoughts on how other npm packages do this? Packages whose client code is DIFFERENT than their server code and who have reasons for using both in various contexts?

@marcoscaceres
Copy link
Contributor

Hmm... I don't know actually. That's a good question, but I don't know what to suggest.

@joshgoebel
Copy link
Member

I was kind of hoping there was some convention everyone else is doing (that makes sense) that we could just point to and follow. :-) I feel like we're unique by having two "targets" (browser and node.js) and that makes us different than a lot of the simpler examples I've seen (so far).

@marcoscaceres
Copy link
Contributor

The only similar ones I can think of is projects that ship with a "-cli"... like babel and babel-cli. So you can get the babel library, and you can get the command line tool separately?

@noraj
Copy link
Contributor

noraj commented Dec 21, 2019

I see an example: FA.
They have only one repository for free fonts https://github.com/FortAwesome/Font-Awesome

But they have several npm packages in https://www.npmjs.com/~robmadole

  • @fortawesome/fontawesome-free-solid
  • @fortawesome/fontawesome-free-brands
  • @fortawesome/fontawesome-free-regular
  • @fortawesome/fontawesome-free-webfonts

Those packages are deprecated and now move into:

  • @fortawesome/fontawesome-free
  • @fortawesome/ffree-solid-svg-icons
  • etc.

By taking a look at https://github.com/FortAwesome/Font-Awesome/blob/master/UPGRADING.md#50x-to-510 you can see they are using something called npm scope

More about scope:

Also look at their way to store multiple npm packages in one single git repository: https://github.com/FortAwesome/Font-Awesome/tree/master/js-packages/%40fortawesome

I hope this could help.


Totally different subject: Configuring npm for use with GitHub Packages

@joshgoebel
Copy link
Member

joshgoebel commented Dec 21, 2019

@noraj Interesting, but is that directly comparable? The problem is we have two (at least) entirely different deliverables -- packaged very differently... the browser/CDN build and the node.js build. Currently our npm is the node build.

Isn't font awesome really just all client-side resources? IE, it's more like our CDN build... simply publishing the CDN build onto npm under another name was one of the ideas mentioned above.

@noraj
Copy link
Contributor

noraj commented Dec 22, 2019

@noraj Interesting, but is that directly comparable? The problem is we have two (at least) entirely different deliverables -- packaged very differently... the browser/CDN build and the node.js build. Currently our npm is the node build.

Yep. I thought you wanted to make it like that:

  • If you want to use Highlight.js server-side, use highlight-js package (for node)

  • If you want to use Highlight.js assets client-side, use highlight-js-assets package (for browser/CDN)

One npm package per usage/target. So just add in the git repository a structure like that:

hljs
└── js-packages
    └── @highlightjs
        ├── highlight-js
        │   └── package.json
        └── highlight-js-assets
            └── package.json

Where @highlightjs is the npm scope. Then you can use symbolic links to point some already existing files from the root git repository into each package sub-directory or build them with a gulp task for exemple in case of the webpack for the browser/CDN package.

So no it's not totally comparable to FA but most of the packaging and delivery idea is here The only difference is that they package logically different stuff and you want to package the "same" stuff for two different targets with some variances.

@joshgoebel
Copy link
Member

Yep. I thought you wanted to make it like that:

Not sure but that's what I'm leaning towards.

So just add in the git repository a structure like that:

I don't think we should add build artifacts to our repository though... I'm not sure what's with that as a trend. I see some advantages but also seems a very annoying thing. We already had the chance to do that with our CDN assets and the original author chose to make a whole separate repo, so I'd say he probably agrees. :-)

@rajsite
Copy link

rajsite commented Dec 23, 2019

Following up on #678 (comment)

If a package.json was included in https://github.com/highlightjs/cdn-release that would work well enough for a personal project I'm working on. The package.json can be dead simple, just needs a name and version.

As an example I forked cdn-release, and created a new branch from tag 9.17.1 called 9.17.1-with-package: https://github.com/rajsite/cdn-release/tree/9.17.1-with-package

The package.json content was:

{
  "name":"@rajsite/highlightjs-cdn-release",
  "version": "0.0.1",
  "private": true
}

Where the name I'm using is under a public scope I own (@rajsite) so it won't accidentally conflict elsewhere.

Then I was able to add it to a project using:

npm install git://github.com/rajsite/cdn-release.git#9.17.1-with-package

For what it's worth, I would still prefer a /dist folder in the highlight.js npm package or a separate npm package with the cdn output. I wouldn't use a git based dependency like this for a work project (we have tools to cache public registry packages it would not mesh well with) and it would be a uncommon transitive dependency for other projects to rely on (it won't play well with semantic versioning, etc).

@joshgoebel
Copy link
Member

For what it's worth, I would still prefer a /dist folder in the highlight.js npm package or a separate npm package with the cdn output. I wouldn't use a git based dependency like this for a work project

What if we pushed the CDN repo to npm ourselves under another name (highlightjs-assets, etc), would that not be a good resolution?

@rajsite
Copy link

rajsite commented Dec 23, 2019

For what it's worth, I would still prefer a /dist folder in the highlight.js npm package or a separate npm package with the cdn output. I wouldn't use a git based dependency like this for a work project

What if we pushed the CDN repo to npm ourselves under another name (highlightjs-assets, etc), would that not be a good resolution?

That would make sense to me. It would be nice if the versions matched and LICENSE + README were included as well (a License file is missing right now from what I can tell).

Right now from cdn-release I'm pulling in the min plus a couple of additional languages as separate JS files from cdn git. It's not perfect for code size / number of network requests but helps with getting started and kicking the tires.

separate npm package sounds great! 👍

@joshgoebel
Copy link
Member

That would make sense to me. It would be nice if the versions matched and LICENSE + README were included as well

You mean in the build folder or on the literal CDN?

@rajsite
Copy link

rajsite commented Dec 23, 2019

You mean in the build folder or on the literal CDN?

In the hightlightjs-assets npm package similar to the highlight.js npm package today, ie:

/package.json
/README.md
/LICENSE
/build/hightlight.min.js
...

@joshgoebel
Copy link
Member

joshgoebel commented Dec 23, 2019

Well the actual built package is in build, not the root of the repo... so if we added those files to the package/build that's where they'd end up, not root.

Although we could probably add a root license... a simple PR would probably resolve that. ;-)

9252562

@joshgoebel
Copy link
Member

joshgoebel commented Mar 17, 2020

This is considered resolved by:

https://www.npmjs.com/package/@highlightjs/cdn-assets

If you really want your pre-compiled assets from NPM (whether via npm/yarn or https://unpkg.com or something else) vs some other method, the cdn-assets package is how you should get them.

rajsite added a commit to rajsite/webvi-experiments that referenced this issue Mar 17, 2020
Switch to @highlightjs/cdn-release for builds highlightjs/highlight.js#678
@rajsite
Copy link

rajsite commented Mar 17, 2020

Worked great for me! Thanks! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package/build Issues relating to npm or packaging
Projects
None yet
Development

Successfully merging a pull request may close this issue.