Rewrite / 1.0

[JedWatson]

I'm midway through a massive rewrite of this library and wanted to open an issue so it's more visible. There will be several breaking changes to the API, all for the better.

WIP is visible on the rebuild branch.

Aside from how we're bundling lib and dist files (which won't change for now), it basically addresses all the things that @brianreavis raised in #227.

I've been trying to split Select up into more granular components as suggested by @bruderstein in #396, but it's not obvious how to do this while also cleaning everything up and preserving a simple API so that can probably be the focus of another major version down the track.

The big improvements include:

• Everything is simpler (I'm nearly done and the source code is only 60% of the size of the last version)
• No more timeouts or weird handlers, the restructuring has let me make everything more straight-forward
• The options array is no longer preprocessed into state, just retrieved from props
• The values array is now initialised in the Options array during render, and not stored in state, which along with the change to options makes the component more reliable and fixes issues with props not updating correctly
• The component no longer stores its own value in state (ever) - it needs to be passed as a prop and handled with onChange.
• Complex values are now enabled by default (so you're passed the option object, not its value); you can enable the legacy mode with a prop
• The Value and Option components have been cleaned up as well for consistency
• The hidden <input> field is now optional and the component is better suited to use in a rich React.js app than it was
• You can disable options filtering to do the filtering externally with onInputChange
• Accents on characters can now be ignored

The other major change is that the whole async options loading piece has been split out into its own Select.Async component, which wraps Select and passes most props through. This dramatically simplifies the select component itself, and plays nicely with the new "options and value are just props" changes. Props that were related to asyncOptions have also been moved to the new component.

Aside from the details of what's been changed, it just feels better to use, more solid. I don't have benchmarks to back this up, but it should be much faster too (and easier to optimise in subsequent versions)

[JedWatson]

Things are looking good so I've merged my progress into master. There are going to be a lot of PRs and Issues in the backlog that need review now :)

The examples have all (except tags mode, see below) been rewritten for the new API and everything seems to be working correctly.

There are two new .md documents in the project at the moment; CHANGES where I'm leaving notes for the changelog / upgrade guide; and TODO which is my working list of things left to review.

In major terms, the things left to be done are:

• Pagination support for the new Async component (this wasn't a thing before but I want to finalise the API with support for it before 1.0.0 is released)
• Tags mode is gone, it'll come back, It's the last remaining feature to re-implement
• Custom event handlers were used previously to deal with issues showing and hiding the menu, especially when clicking on the scroll bar. I'm not sure if they're still needed; this needs testing on IE and Windows before I'm sure, but on all the browsers I've got everything's working as expected (Safari / Chrome / Firefox on OS X)
• There are a lot of failing tests; I suspect there is an underlying problem as a lot of the cases actually work in the examples when testing by hand, but once the automated tests are updated they may reveal minor issues to be resolved. Get the tests working with 1.0.0 #571
• Updated documentation and the change log need to be written

[simonhildebrandt]

react-select is very cool and I'm really excited to see the new version - thanks for all your hard work, and for sharing it with us. 👍

[nmn]

I've created a separate issue which I'll delete, but I found some problems with the build process. Specifically, example files weren't recompiling properly.

If you're OK with it, I'd like to contribute the following changes. You could think of it as a complete re-write of the tooling around the project.

• remove the old gulp build process and move to simple npm scripts with new versions of Babel, Browserify etc.
• ignore all the built files from git
• add a .npmignore to include those files for npm (there was a suggestion of using a whitelist but that will be more painful)
• migrate the testing system to use expect-jsx, and airbnb/enzyme. (mocha stays on, wallaby.js I don't know anything about)
• create a simple build system that generates the build files for bower into a separate repo that can live next to this one. (a bit of a hack, but it'll work.)

Other than all that, I'm working to make things more flexible, and friendly for developers. You know, DX!

• allow users to pass in customRenderLabel methods as a child instead.
  So, instead of:

<Select
    {...otherProps}
    optionRenderer={this.renderOption} />

You can do this:

<Select {...otherProps}>
    <Option>
        {this.renderOption}
    </Option>
</Select>

This means the internal implementations of Option etc, should be exposed to the users.

Additionally, later, I'll work to make it possible to override the inbuilt Option component altogether. As long as you create your own Option component, that has the same displayName, it will be used instead of the built in Option.

• Make more parts of the UI customizable. For example, the entire OptionsMenu could be it's own component, which can be overridden.
• Make it possible to add extra UI by accepting additional children. e.g. it should be possible to add a toolbar above the menu that would contain a select all button.
• I also want to do something about CSS. I have good solutions for inline-CSS OR CSS-modules. But I can't think of a good way to use inline CSS without requiring Radium which is a decently big lib for people who don't want to use it.

Please let me know things you want/don't want.

The checked items here are partially done on my computer locally.

[JedWatson]

Hey @nmn, thanks - excited to have you on board and contributing to the project!

remove the old gulp build process and move to simple npm scripts

Sounds good as long as the processes work on Windows as well as OS X / linux - I've had trouble before getting that right and don't want to exclude contributors who have a Windows development environment. Gulp's lasted as long as it has in my projects because it abstracts those difficulties, but I'd love to see what you come up with.

Would be a big win if the process is encapsulated / reusable across multiple projects as the current gulp tasks are.

ignore all the built files from git

+1 but is having a build repo really cleaner? I'm almost more inclined to drop support for bower altogether.

Elemental's had a rocky experience with this.

migrate the testing system to use expect-jsx, and airbnb/enzyme. (mocha stays on, wallaby.js I don't know anything about)

I'm happy for you to make the tests nicer but defer to @bruderstein - he's contributed the bulk of what's there and owns the current tests (is also a Wallaby fan ;)

I like the look of this:

<Select {...otherProps}>
    <Option>
        {this.renderOption}
    </Option>
</Select>

Just to clarify, are you suggesting we deprecate the existing APIs for customRenderLabel etc. and switch, or would this be able to work alongside as an alternative?

The last 3 sound good.

CSS has also been something @jossmac and I have been thinking about how to fix. We've been trending toward JSS - see his recent rewrite of https://github.com/jossmac/react-images

[JedWatson]

Oooh nice, related to bower concerns: mjackson/npm-http-server#17

looks like @nkbt is on it 😀

[bruderstein]

migrate the testing system to use expect-jsx, and airbnb/enzyme. (mocha stays on, wallaby.js I don't know anything about)

wallaby is just a runner, and effectively doesn't play any role, just leave the file in the repo, then anyone with wallaby can have a great testing experience :)

I've not got around to trying enzyme yet, although I've heard mixed reports, but it seems to be getting a lot of attention. It would certainly make some things easier.

If you're mentioning expect-jsx, I assume you're also talking of moving the majority of the tests to the shallow renderer? Whilst I think the shallow renderer is excellent, it doesn't fit for some places, and I think the main Select component is one of them. It's not possible to refactor a component's render method to use sub components, without changing the tests. This breaks a key testing strategy, you can no longer refactor and stay green, you have to change the tests, which go red, and then change the code. Refactoring the code to use sub-components is something that has already occurred in react-select, and the tests were able to (mostly) cope with the change. It wouldn't surprise me if we do it again.

The shallow renderer is however in use for the Select.Async component, where we we're really not interested what Select renders, only the props that are passed to it. There I think it fits really well.

I would really like to stay away from expect-jsx. It only converts the ReactElements to strings, and thereafter produces string diffs. I'm biased, so I'll try and remain objective, but we're using unexpected-react for the shallow renderer tests for Select.Async, and that performs the diffing at the component level, producing much higher quality diffs when something breaks. It can also work with the complete virtual DOM, so you get to test against the same tree you see in the react dev tools. I think converting the existing DOM tests to unexpected-react would be a very worthwhile activity, and would simplify many tests.

For example, the test

it('should filter after entering some text', () => {
    typeSearchText('T');
    var node = ReactDOM.findDOMNode(instance);
    expect(node, 'queried for', '.Select-option:nth-child(1)', 'to have items satisfying', 'to have text', 'Two');
    expect(node, 'queried for', '.Select-option:nth-child(2)', 'to have items satisfying', 'to have text', 'Three');
    expect(node, 'queried for', '.Select-option', 'to have length', 2);
});

Could be rewritten as

it('should filter after entering some text', () => {
    typeSearchText('T');
    return expect(instance, 'to have rendered with all children', 
              <Select>
                   <span className="Select-option">Two</span>
                   <span className="Select-option">Three</span>
             </Select>);
});

And if it fails, it will show a diff with exactly the issue, based on the tree, and not just on the JSX string.
I think this sort of test is much more readable than the existing set, and the error messages are much higher quality. I can imagine we'd find some cases where we'd need more functionality from unexpected-react, but I'm more than happy to add those as and when we find them.

However, as I said, I'm biased, so open to comments & criticism!

[nmn]

@bruderstein I'll do some more research on the testing front.

My original goal, however is not to move everything to expect-jsx. It's more of an all-of-the-above approach that I'm thinking of. expect-jsx for simple shallow unit tests. enzyme for the jsdom based tests and behavior tests.

I'll read up more about unexpected since I missed a lot of its capabilities. It's also familiar enough for future contributors.

@JedWatson

Build Process

The npm scripts that replace gulp will work on windows and *Nix environments alike. As long as you only use nom-based cli-tools everything works anyway. The common things that break are:

• & and && to run multiple commands. nom-run-all is a good package to work around that.
• common unix commands. I've found npm alternatives to all of them:
  • mkdir -> mkdirp
  • rm -rf -> rimraf
• environment variables don't work on windows at the start of commands. So there's cross-env.

The build processes themselves should be easy to use across projects. It will however, probably not be a module that can be npm installed. Instead, I'll make something like a customizable boilerplate that sets up projects for you.

removing built files

I'm almost more inclined to drop support for bower altogether.

That's your call entirely. I don't like bower, and I would love to see it die completely. However, maintaining a separate repo for it can be pretty simple and self-contained. The repo can simply npm install react-select and run it's own scripts. In fact, if we set it up correctly, it should be possible to automate the process completely. A repo that auto pulls the newest version of react-select, bumps its own version number to the same, and runs a few scripts to generate standalone .js files.

Testing

I've written about this above. I'll come to testing.

props.children API

This will most definitely be in addition to the current props based API. But the documentation will probably de-prioritise that. Something I'm looking to do is to be able to mix-and-match sub components for Select.

This is something I'm trying to achieve.

This would use the normal optionList renderer:

<Select {...otherProps}>
    <Option>
        {this.renderOption}
    </Option>
</Select>

But this would use the renderer with list culling for extremely long option lists:

<Select {...otherProps}>
    <Options-Infinity>
        <Option>
            {this.renderOption}
        </Option>
    <Options-Infinity>
</Select>

This would mean that I have refactor these parts into their own components, which may be challenging. But I'm sure it's possible.

CSS

I'll hold off this for now. In general, my plan was to give each component down the tree to have a simple key name. And then Select should be able to take a deep option of either inline-styles OR classNames for CSS-modules.

The module itself can ship with a few separate themes that way, but it's tricky to do it without inline-styles.

If there wasn't a need for pseudo selectors like :hover, the whole thing would be quite easy. I just don't want to include Radium for people who don't use it.

Lodash?

What do you feel about adding a dependency on lodash? It would make like a little simpler when dealing with the child APIs.

[nmn]

BTW, I'm creating a WIP Pull request now.

[jamiebuilds]

In regards to styles, a pattern that makes things really nice is to rely on composition for it. Basically, have a core component that allows classnames or styles to be specified, but specifies nothing (or next to nothing) by default. Then wrap that in a presentational component that specifies either default classes or styles.

This allows people to just use the component's logic and make the presentational layer themselves. For an example of this see react-modal2 which is the business logic and cf-component-modal which is an example presentational component that uses react-modal2 internally by simply specifying classnames.

[nkbt]

One more note on styling publishable UI components: https://github.com/istarkov/babel-plugin-webpack-loaders

Using this plugin + css-loader + css-modules allows to publish UI components with internal styles (with unique clasnames by css-modules), meanwhile allowing to override any possible classname

[colinmegill]

@JedWatson @nmn I would help convert this to Radium, need it for a project but don't want to include CSS in the pipeline. Let me know if you go that way.

[nmn]

The CSS part was the major blocker for us. Trying to provide something that works for everyone is quite hard.

[jossmac]

@colinmegill we're leaning towards react-jss and possibly react-themeable. Would you still be interested in aiding conversion?

[jamiebuilds]

I think its a really poor decision to use any of those. The best React components are the ones that don't have these kinds of opinions and advocate wrapping.

[colinmegill]

Could you elaborate James?

On Thu, Jan 28, 2016 at 5:09 PM James Kyle [email protected] wrote:

I think its a really poor decision to use any of those. The best React
components are the ones that don't have these kinds of opinions and
advocate wrapping.

—
Reply to this email directly or view it on GitHub
#568 (comment)
.

[jamiebuilds]

@colinmegill Sure, the React community certainly hasn't standardized around a particular way of authoring styles. You could argue that the standard is inline styles (which certainly doesn't describe everyone), but even then there are dozens of ways of doing that.

When you pick one particular way of doing styles in a general purpose component then you are locking people out of doing what they want for very little gain.

If you design your general purpose components to be wrapped in other components by users or more opinionated components it becomes far more pleasant to use (composition is a really nice pattern).

With styles you do this by allowing the user to specify classnames or inline styles for your component, then the user can wrap it however they want and use the wrapped version (which also aids in being able to swap out the underlying component without changing your own interface).

[nmn]

@thejameskyle I've been thinking along those lines, but I've been having a hard time figuring out the actual details. That said, I'm sure there's a way to keep things flexible and move forward with a solution that works for everyone. Just not there yet.

[jamiebuilds]

I've written a module which I hope might help with this problem: https://github.com/thejameskyle/react-stylish

[nmn]

@thejameskyle That's a great module, and mostly solves all the problems. The one edge case for me is Radium. How can we conditionally require Radium only when using inline styles? Without it pseudo selectors won't work. And it's a large enough library that requiring it for everyone is a No-No. That is basically the blocker for me right now.

[jamiebuilds]

I'm just reading about how Radium works, I'll dive into it tonight but I think this might work: https://github.com/thejameskyle/react-stylish/issues/3

I also just published a blog post about this

[colinmegill]

Radium has gained a lot of traction in the community, if you haven't read about it yet be sure to understand what's it's doing first - happy to jump on a google hangout with everyone if that's helpful.

It has proven to be quite smooth for publishing consumable components. https://github.com/FormidableLabs/victory-pie/blob/master/src/components/victory-pie.jsx

In a nutshell: styles passed in as props override default styles owned by the component. Object.assign({}, styles.ownedByComponent, styles.passedInByUser)

Super simple

[jamiebuilds]

@colinmegill This is probably not the best place to get into this discussion

[colinmegill]

Fair enough I'll hold off - @jossmac I'm not crazy about them. Check out the comparisons of libs here: https://github.com/FormidableLabs/radium/tree/master/docs/comparison if you decide on Radium reach out and I'll pitch in.

[nmn]

@colinmegill can I join you on that hangout if you don't mind!

[jhollingworth]

Tags mode is gone, it'll come back, It's the last remaining feature to re-implement

Does anyone know whats happening here? It's become a bit of a blocker for us... We're happy to spend some dev time on this if we understood what needs to be done.

[kumarharsh]

@nmn use the <Style> tag offered by radium, and write your css like so:

'.Select.has-value.is-pseudo-focused:not(.Select--multi) > .Select-control > .Select-value .Select-value-label': {
      color: palette.white,
},

'.Select.Select--multi .Select-input, .Select.Select--multi.has-value .Select-input': {
      marginLeft: 0,
},

[kumarharsh]

Also, really really need the tags functionality. I was mildly shocked to see that the docs were there, and after spending an hour debugging my code, I found out that the allowCreate code was commented out in the source :(

[dherran]

Lads, I've been working for the past few week with React Select and I love it. I've sent a couple of PRs there. I am willing to pitch in some time to help close some issues, work on bugs, missing functionality, test builds, etc. I can also polish those examples, add new ones and update the docs if needs be.

[jxodwyer]

@JedWatson similar to @kumarharsh I'd love to bring back tagmode for v1.0. Is there any specific reason why this feature hasn't been implemented yet? I'd be happy to help out with implementing it for the upgrade.

[bruderstein]

@jxodwyer tag mode is there, see the multiselect demos on http://jedwatson.github.io/react-select/

AFAIK, the only thing that isn't is an allowCreate option, but you can easily emulate that by adding a custom filter, and appending a { value: 'created', label: 'Create New', create: true } to the list of options. Then you can just check when that option is selected.

I looked at adding an allowCreate option back in with the new model, but it turned out it wasn't really doing anything other than what I've just explained. Maybe we should add this to the docs.

[jxodwyer]

@bruderstein thanks a lot for the response! You were right that the allowCreate option was exactly what I was looking for. As you mentioned, overriding the filterOptions func did indeed give me the ability to accomplish this. 👍

[ingro]

Sorry to write here @bruderstein but I have an issue with your method, maybe I'm missing a step.
I'm using an Async Select and I have successfully returned the new option within filterOptions, but when I press enter I don't see the created option in the input, even if the input value is updated correctly.

As a workaround I did this in the onChange handler, but it feels very dirty:

    handleChange(option) {
        if (option && option.create) {
            this.refs.select.setState({
                options: [{ value: option.value, label: option.value }]
            });
        }

        this.props.onChange(option);
    }

Any better way to handle this case?

[bruderstein]

The option has to be there - as it's a controlled component, the caller is responsible for ensuring that the option is created. So you'd just need to ensure that if you get the option with the { create: true } property, you'd create it (in whatever way you need for your options), and include it in the returned options from filterOptions. Then, setting the value to this value should show it correctly.

[nhducit]

I'm looking for a way to render customize label, I can copy and modify the "Value" component. But it will be hard if I want to update to further version. I think @nmn 's solution will be more flexible and can solve my problem. May I know which branch and progress which you guys are working on custom render label?

After dig in to source code I found "valueRenderer" function and it solves my problems.

[tannerlinsley]

@JedWatson Where are you guys with a stable 1.0.0 release? I would be more than willing to help. I've been using the old stable for a while now and would love to upgrade @nozzle to 1.0.0

[TheSharpieOne]

Discussion continued: #1481

[agibralter]

Is there any chance of #59 making it into 1.0.0?

[JedWatson]

1.0.0 is out 🎉

[damianobarbati]

@JedWatson is JSS supported?
My team hasn't been using/importing .css for quite a long time since we started using JSS (and Material-UI) xD