Feat: Add cssAttributes jsAttributes options (resolve #5)
#11
Conversation
- Add `generateAttributes` utility method - Add test for `jsAttributes` - Refactoring `generateCSSReferences` `generateJSReferences` - Update readme to reflect the changes
bebraw
left a comment
bebraw
left a comment
There was a problem hiding this comment.
Looks like a cool idea to me. Let's see what Artem thinks. 👍
sapegin
left a comment
sapegin
left a comment
There was a problem hiding this comment.
This looks like a good idea, yeah!
A few suggestions:
- Rename
jsAttributesandcssAttributesingenerateJSReferencesandgenerateCSSReferencestoattributes, it's obvious from the function name. - Make this props accept object without the need for
generateAttributes.
- `generateCSSReferences` and `generateJSReferences` now accept an object without the need to use `generateAttributes` utility - Fix `publicPath` option (didn't work) - Add test for `publicPath` - Update readme to reflect the changes above
sapegin
left a comment
sapegin
left a comment
There was a problem hiding this comment.
Thanks! One minor comment, otherwise looks good to me 🦄
The main question for me now: is this a breaking change, or we can release it as a minor version?
| jsAttributes = '', | ||
| attributes = {}, | ||
| }) { | ||
| attributes = generateAttributes(attributes); |
There was a problem hiding this comment.
Let's not shadow the function argument to make code easier to understand.
There was a problem hiding this comment.
Thanks @sapegin! In the current release v1.0.0, generateCSSReferences accept two arguments (css, publicPath), in this pull request I changed it to accept an object ({files: css, attributes: cssAttributes, publicPath}). Same situation for generateJSReferences. This is a breaking change for me. Do you think it should be released under v1.1.0 or 2.0.0?
|
Yeah, let's do 2.0.0 to be safe 🔒 |
|
I propose another change. If you set |
## New features * Add `attributes` option to `generateJSReferences` and `generateCSSReferences` (resolve [#5](#5)) ([#11](#11) by @pldg) ### Breaking changes * `generateJSReferences` accepts an object `({files: css, attributes, publicPath}` instead of two arguments `(js, publicPath)`. * `generateCSSReferences` accepts an object `({files: css, attributes, publicPath}` instead of two arguments `(css, publicPath)`.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
generateAttributesutility methodjsAttributesgenerateCSSReferencesgenerateJSReferences