Open dutiyesh opened 4 years ago
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
Right. We can remove this feature.
Regarding "Feature 1: CDN link on GitHub repository", what are your thoughts on Proposal 2 and 3?
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.
Feature 1 - Use
raw.githubusercontent.com
and read everything frompackage.json
. Usejsdelivr
,cdn
,browser
, andmain
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?
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
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.
@MartinKolarik I have few doubts in resolving default file name:
browser
and main
field priority orderDoes 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",
.min
in default file nameAlong with .min
in file name, do we also perform below operations:
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
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?
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.
Yes, we can use our API.
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/jquery@4.0.0-pre
) 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.
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.
Sure. Let's get the version from API itself.
Is there anything missing from what we discussed or can I test and re-review everything now?
Everything is done. I will test it once again.
@MartinKolarik I was thinking of making this extension cross-browser compatible using WebExtension API. What do you think?
👍 I was thinking that could be done at some point. If it's easy enough, you can definitely do it right away.
Awesome!
I've used web-ext and webextension-polyfill to make a FF/Chrome/Chromium based extension fairly easily, if that's any help.
Thank you @frehner , using same libraries. 👍
@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?
Maybe a small pretty tooltip explaining what the plugin does? Otherwise it would be too confusing
True. Will think of some design.
@dutiyesh Hey, how are things going? Do you have time to maintain this addon?
@jimaek I'm doing fine. And yes, I have time to maintain this addon.
Great, can you add the missing features, re-test and finalize it please?
Sure, I will do it.
@jimaek How is this design for tooltip?
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
Right, statement can be updated.
@jimaek I have created a pull request https://github.com/jsdelivr/extension-chrome/pull/12 for above Tooltip, let me know if there are any modifications required in design.
This is a discussion on jsDelivr Extension features and how we can implement it.
Features
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 URLGitHub 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 bejquery
.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 asslick
. But its name defined inpackage.json
file isslick-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'spackage.json
file from jsDelivr CDNWe 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 requireuser
,repo
andversion
. We can getuser
andrepo
from URL, but notversion
parameter, so we will have to call an API to getversion
and then fetchpackage.json
file.The steps will be: Step 1: Get
version
fromv1/package/gh/:user/:repo
API Step 2: Fetchpackage.json
file Step 3: Readname
,version
and defaultfile
name frompackage.json
file Step 4: Generate a CDN URLAnd 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 evaluateversion
parameter to be1.5.8
.Proposal 3: Get
package
name by fetching repository'spackage.json
file from GitHub repositoryWe can fetch a RAW
package.json
file from GitHub repository itself.The steps will be: Step 1: Fetch RAW
package.json
file fromhttps://raw.githubusercontent.com/:user/:repo/master/package.json
file Step 2: Readname
,version
and defaultfile
name frompackage.json
file Step 3: Generate a CDN URLAnd if a user wants a specific version of project from GitHub's Release tags, we can get
version
from its GitHub URL and fetch itspackage.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 RAWpackage.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 andversion
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
andversion
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.