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

Replace reference to Skypack with jsDelivr #148

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

afcapel
Copy link
Contributor

@afcapel afcapel commented Sep 22, 2023

Skypack fails to compile packages that use private class fields, even though they are already supported by all major browsers and node. jsDelivr is convenient in that it provides automatically minified versions of the dist files.

Ref.

https://caniuse.com/mdn-javascript_classes_private_class_fields

skypackjs/skypack-cdn#308
skypackjs/skypack-cdn#222
skypackjs/skypack-cdn#351
skypackjs/skypack-cdn#354

Skypack fails to compile packages that use private class fields, even
though they are supported by all major browsers and node.

Ref.

skypackjs/skypack-cdn#308
skypackjs/skypack-cdn#222
skypackjs/skypack-cdn#351
skypackjs/skypack-cdn#354

```html
<head>
<script type="module" src="https://cdn.jsdelivr.net/npm/@hotwired/turbo@latest/dist/turbo.es2017-esm.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.

Would encouraging consumers to use ga.jspm.io be appropriate?

When I execute bin/importmap pin @hotwired/turbo@latest in a Rails project using Importmaps, it adds the following entry to config/importmap.rb

pin "@hotwired/turbo", to: "https://ga.jspm.io/npm:@hotwired/[email protected]/dist/turbo.es2017-esm.js"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any strong preference for any CDN, as long as they work. I think it's ok to mix and match, we're not officially endorsing any.

Copy link
Contributor

@kevinmcconnell kevinmcconnell left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Could also add a link to the package page, as well as showing the script tag example. But seems good either way.

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

Successfully merging this pull request may close these issues.

3 participants