-
Notifications
You must be signed in to change notification settings - Fork 31
change deps #33
change deps #33
Conversation
Then fallback to global: `window`.
Travis failed because of rollup config (since it actually thinks it's |
Bundle needs to happen on |
Hey @stevemao just got to merging your other PRs. Can you have another look at this one when you have a chance pls? Thanks! |
Yes, of course. |
* master: fix(context): bind the right context to prototype functions feat(proto): expose web component proto methods onto react component Revert "test(proto): add a test for props on prototype" Revert "feat(proto): expose web component proto props onto react component" test(proto): add a test for props on prototype feat(proto): expose web component proto props onto react component fix(event): on[upper]* for event name syntax docs(README): clarify react component
@@ -1 +1,13 @@ | |||
module.exports = require('skatejs-build/rollup.config'); | |||
const config = require('skatejs-build/rollup.config'); |
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.
👍
Do you think it'd be a good idea to ignore peer dependencies here, as well? https://github.com/skatejs/build/blob/master/rollup.config.js#L24
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.
why? skatejs-build doesn't have peer dependencies?
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.
@treshugart should we put this logic in https://github.com/skatejs/build?
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.
I know it doesn't have peer deps, but the build uses the cwd.
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.
It'd be ideal to be put in there, I think. If you want to do that as part of this, I think that would be best.
I don't have a lot of context to this but it looks pretty weird. It looks like it should work though. |
lgtm |
@bradleyayers the idea is that react and react dom can be required as they'd be peer dependencies and provided by the consumer anyways. This build stuff can eventually be contained in skatejs/build (as it already handles |
lgtm |
LGTM |
lgtm |
lgtm! |
lgtm |
chore(deps): add missing devdeps
feat(deps): require react and reactDOM since they are peerdeps