[image-url] Introducing the image-url package #352
Conversation
| @@ -0,0 +1,38 @@ | |||
| { | |||
| "name": "@sanity/image-url", | |||
| "version": "0.119.0", | |||
| "lint": "eslint .", | ||
| "pre-commit-check": | ||
| "npm test && (eslint . || (echo 'Warning: project has lint errors. Please fix and re-commit with `git commit --amend`' && echo))", | ||
| "test": "mocha --recursive --require babel-register test" |
There was a problem hiding this comment.
babel-register should probably be added to dev dependencies?
There was a problem hiding this comment.
Probably want to add eslint with Sanity config and a lint task
| "babelify": "^7.3.0", | ||
| "browserify": "^14.3.0", | ||
| "mocha": "^3.2.0", | ||
| "remon": "^1.0.2", |
There was a problem hiding this comment.
Don't think remon is used in this project?
There was a problem hiding this comment.
Neither is browserify/babelify if the bloat task is removed
packages/@sanity/image-url/.babelrc
Outdated
| @@ -0,0 +1,10 @@ | |||
| { | |||
| "presets": ["es2015", "react"], | |||
There was a problem hiding this comment.
The react preset should be removed from here
packages/@sanity/image-url/.babelrc
Outdated
| "transform-object-rest-spread", | ||
| "syntax-class-properties", | ||
| "transform-class-properties", | ||
| "lodash" |
There was a problem hiding this comment.
As far as I can see, neither of these plugins are actually needed and should also be removed.
| @@ -0,0 +1 @@ | |||
| /lib | |||
There was a problem hiding this comment.
You'd probably want to add node_modules here too
packages/@sanity/image-url/.eslintrc
Outdated
| "sanity", | ||
| "sanity/react" | ||
| ], | ||
| "parser": "babel-eslint" |
There was a problem hiding this comment.
I don't think babel-eslint is needed
| @@ -0,0 +1,4 @@ | |||
| example | |||
| npm install --save @sanity/image-url | ||
|
|
||
| ## Usage | ||
|
|
There was a problem hiding this comment.
Maybe write a bit more on this part? :)
packages/@sanity/image-url/index.js
Outdated
| @@ -0,0 +1 @@ | |||
| module.exports = require('./lib') | |||
There was a problem hiding this comment.
IMO ./lib/index.js is an unnecessary indirection, and it would be better to skip it, and instead require ./lib/builder.js directly here. Also i think there's an issue with mix of export / commonjs requires here that can be avoided with:
module.exports = require('./lib/builder').default| "scripts": { | ||
| "clean": "rimraf lib", | ||
| "bloat": | ||
| "browserify -t babelify --full-paths ./src/index.js | discify --open", |
There was a problem hiding this comment.
The "bloat" script here is not that useful and can be removed (I also don't think it will work since disc is not added as a dependency)
| "babelify": "^7.3.0", | ||
| "browserify": "^14.3.0", | ||
| "mocha": "^3.2.0", | ||
| "remon": "^1.0.2", |
There was a problem hiding this comment.
Neither is browserify/babelify if the bloat task is removed
| } | ||
|
|
||
| // Map from spec name to url param name | ||
| [ |
There was a problem hiding this comment.
These could be extracted as a global constant
| @@ -0,0 +1 @@ | |||
| export default require('./builder') | |||
There was a problem hiding this comment.
Instead of mixing ES import / commonjs require you can do export {default} from './builder'. That said, I don't think this file is needed (see my other comment).
Quickly generate image urls from Sanity image records + a builder api for the Sanity image pipeline url options.