-
Notifications
You must be signed in to change notification settings - Fork 921
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
Full "webDependencies" management #236
Conversation
src/config.ts
Outdated
@@ -17,7 +17,7 @@ type DeepPartial<T> = { | |||
// interface this library uses internally | |||
export interface SnowpackConfig { | |||
source: 'local' | 'pika'; | |||
webDependencies?: string[]; | |||
webDependencies?: string[] | {[packageName: string]: string}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯! TypeScript is so nice. It’s hard to remember what supporting different data structures for backwards compatibility was like before typed JS
This makes sense to me. +1 for the object format you went with. In general maps are great, easy structures to work with. And mirroring
What is the support story here? Is this an improvement to the |
Yea, this is something new (if webDependencies becomes an object, we still need a way to define multiple entrypoints inside of one package). You're right that we already use
|
Got it. Yeah I’d vote for |
19f7ac0
to
2f7c72b
Compare
rolling #237 conversation into here, we could either continue to support a sub-extension on the types (extending string into [string, string], and similar for objects), or we could support a separate For these reasons, it might be nice if "webDependencies": {
"a": "^1.2.3"
"b": "^1.2.3"
"c": "^1.2.3"
},
"aliases": {
"react": "c"
} We basically need ways to add the alias to the importMap: importMap.imports[alias] = `./${targetName}.js${hashQs}`; add the aliases to rollupPluginAlias({
entries: Array.from(allInstallSpecifiers)
.filter(spec => Array.isArray(spec))
.map(([alias, mod]) => ({find: alias, replacement: mod})),
}), then the |
Just throwing my two cents into the mix here:
Loving the direction snowpack is headed in! It feels like it won't be long before we're a fully-fledged package manager 👀 |
5f437b0
to
c616389
Compare
dccb87b
to
6232a5a
Compare
Thanks for the feedback everyone! Finally picking this back up after leaving it hanging for way too long. PR is now ready for review, with new tests and personally tested on pika.dev. Here's an overview of how the pika.dev package.json changed for the new mode: # Examples package.json
"snowpack": {
- "webDependencies": [
- "@apollo/react-hooks",
- "@emotion/core",
- "@polymer/ristretto",
- "@sentry/browser",
- "@mountaingapsolutions/include",
- "apollo-cache-inmemory",
- "hardtack",
- "apollo-boost",
- "history",
- "react",
- "react-dom",
- "tslib",
- "react-router-dom",
+ "entrypoints": [
"bulma/css/bulma.min.css",
+ "emoji-mart",
"emoji-mart/css/emoji-mart.css",
"github-markdown-css/github-markdown.css",
"highlight.js/styles/default.css"
]
},
- "devDependencies": {
+ "webDependencies": {
"@apollo/react-hooks": "^3.0.1",
"@emotion/core": "^10.0.16",
"@flourish/semver": "^1.0.2",
"@github/textarea-autosize": "^0.1.0",
"@mountaingapsolutions/include": "^1.0.0",
"@polymer/ristretto": "^0.3.1",
"@sentry/browser": "5.7.1",
- "@types/react": "^16.9.2",
- "@types/react-dom": "^16.9.0",
- "@types/react-router-dom": "^5.1.0",
- "algoliasearch": "^3.35.0",
"apollo-boost": "^0.4.4",
- "bulma": "^0.7.5",
- "emoji-mart": "^2.11.1",
"epoch-timeago": "^1.1.9",
"formik": "^1.5.8",
- "github-markdown-css": "^3.0.1",
"hardtack": "^4.1.0",
- "highlight.js": "^9.15.10",
"history": "^4.9.0",
"marked": "^0.7.0",
"qss": "^2.0.3",
- "react": "npm:@reactesm/react",
- "react-dom": "npm:@reactesm/react-dom",
+ "react": "^16.13.0",
+ "react-dom": "^16.13.0",
"react-instantsearch-core": "^5.7.0",
"react-instantsearch-dom": "^5.7.0",
"react-meta-elements": "^1.0.0",
@@ -68,5 +45,16 @@
"react-switch": "^5.0.1",
"tslib": "^1",
"yup": "^0.27.0"
+ },
+ "devDependencies": {
+ "@types/react": "^16.9.2",
+ "@types/react-dom": "^16.9.0",
+ "@types/react-router-dom": "^5.1.0",
+ "algoliasearch": "^3.35.0",
+ "bulma": "^0.7.5",
+ "emoji-mart": "^2.11.1",
+ "github-markdown-css": "^3.0.1",
+ "highlight.js": "^9.15.10"
} Some good learnings from this implementation, to spin off into future discussions:
|
3595638
to
3f2d1bb
Compare
3f2d1bb
to
219d0d9
Compare
Background
Keeping up the momentum from #228: Snowpack still relies on semver info to be stored in your package.json
dependencies
. This messes with our "install from the Pika CDN" story, since that means that npm will still try to install all of your dependencies. For now, all we've really done is added a second, potentially confusing install step.If Snowpack is going to manage your install step for you, that means we need to own the semver data in a place that npm won't try to install from. We've always used the "webDependencies" as a list of local dependencies to install, but in this PR I'd like us to add support for a new, top-level, "
dependencies
-like" semver object.Implementation
--source pika
.assets: []
?include: []
?) A:entrypoints
for all additional entrypoints.Creating this PR now for early feedback, specifically on the public interface changes.
/cc @DangoDev @alex-saunders