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

Extension Features and Implementation #1

Open
dutiyesh opened this issue Sep 22, 2019 · 31 comments
Open

Extension Features and Implementation #1

dutiyesh opened this issue Sep 22, 2019 · 31 comments

Comments

@dutiyesh
Copy link
Contributor

This is a discussion on jsDelivr Extension features and how we can implement it.


Features

  • Provide a CDN link on GitHub and NPM pages
  • Provide an option to download ZIP file on GitHub page

jsdelivr-github-01

jsdelivr-github-02

jsdelivr-npm

Implementation

Feature 1: CDN link on GitHub repository

To form a project's CDN link (https://cdn.jsdelivr.net/npm/package@version/file) we need 3 parameters:

  • package
  • version
  • file

Proposal 1: Get package name from repository URL

GitHub repository URL has below patterns:

  • /:user/:repo
  • /:org/:repo

We can assume that a repository's package name would be same as repository's name and evaluate it from URL.

Example:
For jQuery's GitHub repository with URL - https://github.com/jquery/jquery; its package name would be jquery.

Problems
This proposal fails for scoped package names and when package name differ from their repository name.

Example 1:
For Slick Carousel's GitHub repository with URL - https://github.com/kenwheeler/slick; we would evaluate its package name as slick.
But its name defined in package.json file is slick-carousel.

Example 2:
For a scoped package like - @sindresorhus/class-names; we won't be able to correctly evaluate its package name from its GitHub URL - https://github.com/sindresorhus/class-names

Conclusion: Not a good approach.

Proposal 2: Get package name by fetching repository's package.json file from jsDelivr CDN

We can fetch repository's package.json file from CDN with URL - https://cdn.jsdelivr.net/gh/:user/:repo@:version/package.json, for which we will require user, repo and version. We can get user and repo from URL, but not version parameter, so we will have to call an API to get version and then fetch package.json file.

The steps will be:
Step 1: Get version from v1/package/gh/:user/:repo API
Step 2: Fetch package.json file
Step 3: Read name, version and default file name from package.json file
Step 4: Generate a CDN URL

And if a user wants a specific version of project from GitHub's Release tags, we can skip Step 1 and get version from its GitHub URL.

Example:
For Slick Carousel version 1.5.8, its GitHub URL is - https://github.com/kenwheeler/slick/tree/1.5.8.
From this URL we can evaluate version parameter to be 1.5.8.

Proposal 3: Get package name by fetching repository's package.json file from GitHub repository

We can fetch a RAW package.json file from GitHub repository itself.

The steps will be:
Step 1: Fetch RAW package.json file from https://raw.githubusercontent.com/:user/:repo/master/package.json file
Step 2: Read name, version and default file name from package.json file
Step 3: Generate a CDN URL

And if a user wants a specific version of project from GitHub's Release tags, we can get version from its GitHub URL and fetch its package.json file.

Example:
For Slick Carousel version 1.5.8, its GitHub URL is - https://github.com/kenwheeler/slick/tree/1.5.8.
And its RAW package.json file URL will be - https://raw.githubusercontent.com/kenwheeler/slick/1.5.8/package.json.

Question: Proposal 2 and 3 provides us with default file name as well. So should we use it and rename it with minified file name convention or call jsDelivr API (/package/npm/:name@:version/:structure?) to get the default file name?

Note: Both Proposal 2 and 3 fails for repositories that use Lerna tool. Popular repositories using Lerna: React, Babel.
We have to think for an approach to handle such scenario.


Feature 2: CDN link on NPM

To form a project's CDN link (https://cdn.jsdelivr.net/npm/package@version/file) we need 3 parameters:

  • package
  • version
  • file

Proposal: Get required parameters from NPM URL and HTML page

We can get package name from URL and version from sidebar under "version" heading.
And then get default file name from jsDelivr API - /package/npm/:name@:version/:structure?.


Feature 3: Download ZIP link on GitHub Repository

To generate a project's download link (https://registry.npmjs.org/package/-/package-version.tgz) we need 2 parameters:

  • package
  • version

We can re-use package and version parameters which we got while generating CDN link from Feature 1.
And then generate a download link.

The difference between this link and GitHub's own download link is that it will not have Git instance.

@jimaek
Copy link
Member

jimaek commented Sep 22, 2019

I only have feedback on this: "Feature 3: Download ZIP link on GitHub Repository"
I dont think it makes sense to add it since Github already has a download zip feature by default

@dutiyesh
Copy link
Contributor Author

Right.
We can remove this feature.

@dutiyesh
Copy link
Contributor Author

Regarding "Feature 1: CDN link on GitHub repository", what are your thoughts on Proposal 2 and 3?

@MartinKolarik
Copy link
Member

Feature 1 - Use raw.githubusercontent.com and read everything from package.json. Use jsdelivr, cdn, browser, and main fields in this order to resolve the main file, and add .min if needed.

Feature 2 - Get name from the URL and the rest from npm API (https://registry.npmjs.org/jquery).

Feature 3 - The contents of npm package often differs from the repo so keep this feature.

@dutiyesh
Copy link
Contributor Author

Feature 1 - Use raw.githubusercontent.com and read everything from package.json. Use jsdelivr, cdn, browser, and main fields in this order to resolve the main file, and add .min if needed.

We can do this.
But in future if the logic of resolving the main file changes, we have to ensure that the same changes are made in extension as well.

Feature 2 - Get name from the URL and the rest from npm API (registry.npmjs.org/jquery).

Great idea.

Feature 3 - The contents of npm package often differs from the repo so keep this feature.

Right, npm packages provide build files as well.
Just compared jQuery's GitHub repository with npm. The build files are not committed in repository but are available on npm.
@jimaek , what do you think?

@jimaek
Copy link
Member

jimaek commented Sep 28, 2019

Then it must be clearer I think. First user question will be what's the difference between this zip and the other zip by GitHub

@MartinKolarik
Copy link
Member

Then it must be clearer I think. First user question will be what's the difference between this zip and the other zip by GitHub

It will be mentioned in the readme but I don't think any UI changes are necessary. It simply downloads files that are available at the CDN.

@dutiyesh
Copy link
Contributor Author

@MartinKolarik I have few doubts in resolving default file name:

Doubt 1: browser and main field priority order

Does browser have low priority over main field if its value is of type object?

For example:
axios has defined both main and browser fields in its package file, so based on the priority we will access browser's value, but when I compared it with jsDelivr API's response, the default file name was accessed from main field.
Is it because browser's value is of type object and not string?

package.json

"main": "index.js",
"browser": {
    "./lib/adapters/http.js": "./lib/adapters/xhr.js"
}
jsDelivr API

"default": "/index.min.js",

Doubt 2: Adding .min in default file name

Along with .min in file name, do we also perform below operations:

  1. Remove leading .
    Example: moment package file has main field set to ./moment.js, but jsDelivr API returns a default file name as /moment.min.js

  2. Add leading /
    Example: jQuery package file has main field set to dist/jquery.js, but jsDelivr API returns a default file name as /dist/jquery.min.js

Are there any other operations?


Doubt 3: Inconsistent default file name set in GitHub and npm package files

While comparing default file names set in GitHub package file and npm, I found an inconsistency in js-cookie package.

On GitHub js-cookie's package file has browser field set to dist/js.cookie.js, but on npm it is src/js.cookie.js.

For such case, jsDelivr API would be a good choice over accessing values from GitHub package file and forming a URL.

@MartinKolarik
Copy link
Member

  1. Most likely they changed it recently. I suppose it's best to just use our API, this solves 1) and 2) as well.

@dutiyesh
Copy link
Contributor Author

dutiyesh commented Oct 2, 2019

Yes, we can use our API.

@dutiyesh
Copy link
Contributor Author

dutiyesh commented Oct 2, 2019

I think we have to update our approach.

While testing this pull request against jQuery repository, I found that its version in package file is 4.0.0-pre, but they have not released it yet; so it is not available on jsDelivr CDN.
Due to which our API (https://data.jsdelivr.com/v1/package/npm/[email protected]) fails while retrieving default file name.

New Approach:
Step 1: Fetch RAW package.json file from https://raw.githubusercontent.com/:user/:repo/master/package.json file
Step 2: Read only name from package.json file
Step 3. Get latest version from jsDelivr API (/v1/package/npm/:name)
Step 4. Get default file name from jsDelivr API (/v1/package/npm/:name@:version)
Step 5: Generate a CDN URL

And to optimise these API calls, we can memoize the APIs result.

@MartinKolarik
Copy link
Member

Yes, you are right. I forgot to mention it in my previous comment but it's the same problem as with changed main file which hasn't been published yet. Use our API for the version as well.

@dutiyesh
Copy link
Contributor Author

dutiyesh commented Oct 2, 2019

Sure. Let's get the version from API itself.

@MartinKolarik
Copy link
Member

Is there anything missing from what we discussed or can I test and re-review everything now?

@dutiyesh
Copy link
Contributor Author

Everything is done.
I will test it once again.

@dutiyesh
Copy link
Contributor Author

@MartinKolarik I was thinking of making this extension cross-browser compatible using WebExtension API. What do you think?

@MartinKolarik
Copy link
Member

👍 I was thinking that could be done at some point. If it's easy enough, you can definitely do it right away.

@dutiyesh
Copy link
Contributor Author

Awesome!

@frehner
Copy link

frehner commented Nov 25, 2019

I've used web-ext and webextension-polyfill to make a FF/Chrome/Chromium based extension fairly easily, if that's any help.

@dutiyesh
Copy link
Contributor Author

Thank you @frehner , using same libraries. 👍

@dutiyesh
Copy link
Contributor Author

dutiyesh commented Dec 3, 2019

@MartinKolarik Currently nothing happens when a user clicks on extension icon from toolbar, I was thinking of opening jsDelivr website on a new tab instead.
What do you think?

@jimaek
Copy link
Member

jimaek commented Dec 3, 2019

Maybe a small pretty tooltip explaining what the plugin does? Otherwise it would be too confusing

@dutiyesh
Copy link
Contributor Author

dutiyesh commented Dec 5, 2019

True.
Will think of some design.

@jimaek
Copy link
Member

jimaek commented Feb 11, 2021

@dutiyesh Hey, how are things going? Do you have time to maintain this addon?

@dutiyesh
Copy link
Contributor Author

@jimaek I'm doing fine. And yes, I have time to maintain this addon.

@jimaek
Copy link
Member

jimaek commented Feb 12, 2021

Great, can you add the missing features, re-test and finalize it please?

@dutiyesh
Copy link
Contributor Author

Sure, I will do it.

@dutiyesh
Copy link
Contributor Author

@jimaek How is this design for tooltip?

jsdelivr-popup-2

@jimaek
Copy link
Member

jimaek commented Feb 15, 2021

Looks good but needs an explanation. Maybe something like "Get jsDelivr CDN URLs on npmjs and github project pages."
To make it clear to the user what it actually does

@dutiyesh
Copy link
Contributor Author

Right, statement can be updated.

@dutiyesh dutiyesh mentioned this issue Feb 16, 2021
@dutiyesh
Copy link
Contributor Author

@jimaek I have created a pull request #12 for above Tooltip, let me know if there are any modifications required in design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants