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.

Sign up for GitHub

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

Jump to bottom

add import map support #1545

Merged
merged 2 commits into from
Nov 11, 2020
Merged

add import map support #1545

merged 2 commits into from
Nov 11, 2020

Conversation

FredKSchott
Copy link
Owner

@FredKSchott FredKSchott commented Nov 8, 2020 •
edited
Loading 

$ snowpack add canvas-confetti
[snowpack] fetching canvas-confetti@latest from Skypack CDN...
[snowpack] adding https://cdn.skypack.dev/canvas-confetti@latest to your project lockfile. (snowpack.lock.json)
✨  Done in 0.70s.

Changes

  • Add true import map support via a new snowpack.lock.json file!
  • Remove old "webDependencies" references (these were never documented and were not working)
  • Add and document the add & rm CLI commands to help you generate & manage your lockfile more easily (powered by Skypack CDN).
  • Supports any 3rd party CDN (Skypack recommended/preferred)
  • totally opt-in, no larger changes to how Snowpack works for now (up to you to avoid duplicate dependencies across locally installed packages and CDN packages).
    • @drwpow @melissamcewen this is a bit of a reversal of something that I tried to claim yesterday about not being able to do this piecemeal. I've come around to the idea that a user should be able to opt into the import map idea. The larger "source" feature can come in a later PR / separate feature.

Testing

  • Tested manually
  • add, rm, and lockfile tests all TODO

Docs

  • Docs TODO (this will be documented and released as the next Snowpack minor version).

Future PR

  • Resolver plugin hook to support add/rm on other CDNs? To be fleshed out

@vercel
Copy link

vercel bot commented Nov 8, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/p37wo4upv
✅ Preview: https://snowpack-git-wip-lockfile.pikapkg.vercel.app

@FredKSchott FredKSchott changed the title add true snowpack.lock.json support add import map support Nov 8, 2020
drwpow
drwpow reviewed Nov 10, 2020
View reviewed changes
skypack/src/index.ts
@@ -196,7 +196,10 @@ export async function lookupBySpecifier(
(semverString ? `@${semverString}` : ``) +
(packagePath ? `/${packagePath}` : ``);
try {
const {body, headers, isCached, isStale} = await fetchCDN(lookupUrl, userAgent);
const {body, statusCode, headers, isCached, isStale} = await fetchCDN(lookupUrl, userAgent);
if (statusCode !== 200) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any other situation where we‘d return another 2xx code? (or 3xx?)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Great call, we do only return 200s in this case, but this is one place where the ?meta param will really help

drwpow
drwpow reviewed Nov 10, 2020
View reviewed changes
snowpack/src/config.ts
@@ -81,10 +81,6 @@ const configSchema = {
install: {type: 'array', items: {type: 'string'}},
exclude: {type: 'array', items: {type: 'string'}},
plugins: {type: 'array'},
webDependencies: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉

drwpow
drwpow reviewed Nov 10, 2020
View reviewed changes
snowpack/src/util.ts
import {ImportMap, SnowpackConfig} from './types/snowpack';

import type {HttpieResponse} from 'httpie';

export const PIKA_CDN = `https://cdn.pika.dev`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉 🎉

@drwpow
Copy link
Collaborator

drwpow commented Nov 10, 2020

I love this!

Not as a part of this PR, but yeah would love to get some docs in for this, linking to the official import map proposal and all. Always exciting to introduce people to more emerging standards through a tool they commonly use

@FredKSchott FredKSchott merged commit 76d3e2b into master Nov 11, 2020
@FredKSchott FredKSchott deleted the wip-lockfile branch November 11, 2020 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drwpow drwpow drwpow approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FredKSchott @drwpow