developit / preact-redux

:loop: Preact integration for Redux (no shim needed!)
https://npm.im/preact-redux
MIT License
288 stars 26 forks source link

Improve documentation or maybe the code itself #9

Closed piuccio closed 6 years ago

piuccio commented 7 years ago

Hi thanks for the package, but I think there's something wrong with the examples.

In the README you say

import { Provider, connect } from 'preact-redux';

but it should probably be

import redux from 'preact-redux';
redux.connect();

That's because in rollup config you only export default.

Maybe a better way is to change that line to

import * as lib from './index'; export default lib; export { Provider, connect, connectAdvanced } from './index';

The reason I'm saying this is because in webpack I get the error "export 'Provider' was not found in 'preact-redux'

Also it's a bit weird that you're transpiling all the way down to ES3. Would be nice to find a solution to developit/preact#269 and standardise what ES version is exported by module or jsnext:main, honestly I'd like to have a way to use ES6 directly, no transpilation. I can then babel-ify my node modules according to the browsers I'm going to support.

developit commented 7 years ago

The reason this isn't supposed is because it's not possible to use this module via jsnext:main/modules. The purpose of this module is actually to pull in react-redux, strip a couple things out, and pre-alias it. That can't be done if the dist files are bypassed and src is used. This module is basically just an aliasing build applied to the upstream react-redux dependency, so without that aliasing build it doesn't do anything at all.

That being said, I'd be totally open to adding an ES Modules pre-aliased bundle to dist/ and marking that as jsnext:main/modules in the package.json. That allow for proper ES6 usage, but still preserve the aliasing of react-redux. Thoughts?

For the webpack error - you must have had to add an alias or jsnext:main/modules in order to get to that state? Currently only a UMD bundle is exported, which should never trigger that warning.

BTW - rollup-plugin-es3 just removes Object.freeze() calls from the exports, it doesn't do anything else special for es3. It's an odd name 😆

piuccio commented 7 years ago

Ok for src files not to be exposed, but named exports could still be useful. Not that it makes any difference in webpack, I have the feeling that tree shaking is doing very little.

developit commented 7 years ago

The tree shaking is being bypassed intentionally, yeah. Thanks for the PR!