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

Add CDN links #8980

Merged
merged 1 commit into from
Oct 3, 2017
Merged

Add CDN links #8980

merged 1 commit into from
Oct 3, 2017

Conversation

LukasDrgon
Copy link
Contributor

I added jsDelivr CDN links to your site and readme for easier and faster usage. jsDelivr is the fastest opensource CDN available and built specifically for production usage.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Sep 30, 2017

I'm not sure that we, first of all, should (effectively) endorse a specific CDN in this very visible way!?
Secondly, even if we do this, I don't really think that it belongs in https://mozilla.github.io/pdf.js/getting_started, since that page should contain just the bare minimum.
Finally, if added to README.md, it shouldn't be placed in such a way that it takes precedence over the existing "Getting the Code" section (since that is the only thing we can really guarantee works).

Edit: Also, does following these instructions work correctly? Considering that there's no mention of the pdf.worker.js file, and specifically the fact that PDFJS.workerSrc should be set to point to it.

@LukasDrgon
Copy link
Contributor Author

I'm not sure that we, first of all, should (effectively) endorse a specific CDN in this very visible way!?

That is, of course, completely up to you. Most JS projects do provide a CDN link in their README/documentation because it makes using the code easier (and using a CDN is usually better than serving files from your server).

From user's POV, I can say that the getting started page is exactly where I would expect this, in addition to the full zip packages, but I'll be happy to modify this PR if you decide you actually want this somewhere.

@jimaek
Copy link

jimaek commented Sep 30, 2017

Interestingly enough Mozilla has covered jsDelivr before :) https://hacks.mozilla.org/2014/03/jsdelivr-the-advanced-open-source-public-cdn/

@yurydelendik
Copy link
Contributor

yurydelendik commented Oct 1, 2017

What jsDelivr provides that cdnjs or npmcdn does not? Does jsDeliver allow cors to access cmaps?

@MartinKolarik
Copy link

@yurydelendik

  • Multiple CDN providers, including one in China, which results in better load times.
  • Also multiple DNS providers, origin server locations, and backup storage system - it's visually explained here and means that even if half of the services we use went down at the same time, jsDelivr would still continue to work.
  • Support for on-demand minification and bundling of multiple JS/CSS files.
  • Usage statistics per project/version/file (some developers actually use this to drive certain development decisions).
  • Similarly to npmcdn, instant updates from npm with zero config, and semver version ranges for auto-updating URLs, such as /npm/pdfjs-dist@1

Does jsDeliver allow cors to access cmaps?

Yes, cors is enabled for all resources.

@yurydelendik
Copy link
Contributor

yurydelendik commented Oct 1, 2017

Does jsDeliver allow cors to access cmaps?

Yes, cors is enabled for all resources.

I tested the https://gist.github.com/yurydelendik/016016dcb126ee5c84b4a9c3cc734d8e example. The CORS at jsDeliver does not work. I tested npmcdn.com it works there.

Copy link
Contributor

@yurydelendik yurydelendik left a comment

Choose a reason for hiding this comment

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

We want to provide an alternative CDNs and don't want to be exclusive to the jsDelivr. Please rephrase text in neutral language and list available/popular CDNs that host PDF.js at this moment.

README.md Outdated
@@ -45,6 +45,13 @@ PDF.js is built into version 19+ of Firefox, however, one extension is still ava
Chrome, go to `Tools > Extension` and load the (unpackaged) extension from the
directory `build/chromium`.

### Including via a CDN

To use PDF.js via a CDN you need to include this file from [jsDelivr](https://www.jsdelivr.com/package/npm/pdfjs-dist):
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this section as ## after the "Building PDF.js"

README.md Outdated

To use PDF.js via a CDN you need to include this file from [jsDelivr](https://www.jsdelivr.com/package/npm/pdfjs-dist):
```
<script src="https://cdn.jsdelivr.net/npm/pdfjs-dist@1/build/pdf.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot exclusively list only jsdelivr, please list npmcdn.com prior jsdelivr.net since some finality (e.g. cmaps) does not work with the former.

@@ -37,6 +37,12 @@ Before downloading PDF.js please take a moment to understand the different layer

## Download

You can use PDF.js by including this file from [jsDelivr](https://www.jsdelivr.com/package/npm/pdfjs-dist):
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the text from the download section, list it in the separate section below.

@MartinKolarik
Copy link

MartinKolarik commented Oct 1, 2017

I think you just used a wrong link there. https://www.jsdelivr.com/ is the website, CDN files are on a different domain (for security and other reasons) - https://cdn.jsdelivr.net/npm/pdfjs-dist/cmaps/

@yurydelendik
Copy link
Contributor

I think you just used a wrong link there. https://www.jsdelivr.com/ is the website,

Sorry, I just copied what is/was provided at a8d7397#diff-04c6e90faac2675aa89e2176d2eec7d8R50

@LukasDrgon
Copy link
Contributor Author

@yurydelendik Let me know if you want to change anything else.

README.md Outdated
@@ -88,6 +88,13 @@ This will generate `pdf.js` and `pdf.worker.js` in the `build/generic/build/` di
Both scripts are needed but only `pdf.js` needs to be included since `pdf.worker.js` will
be loaded by `pdf.js`. The PDF.js files are large and should be minified for production.

## Including via a CDN
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this section after ## Using PDF.js in a web application?

@timvandermeij
Copy link
Contributor

@LukasDrgon Also, please squash the commits into one commit (see https://github.com/mozilla/pdf.js/wiki/Squashing-Commits) once you made the change requested above.

Add CDN link

Add Popular CDNs

Add popular CDNs (site)

Moving section
@LukasDrgon
Copy link
Contributor Author

All done 👍

@timvandermeij timvandermeij merged commit e279d37 into mozilla:master Oct 3, 2017
@timvandermeij
Copy link
Contributor

Thank you for the patch!

movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants