JedWatson / react-select

The Select Component for React.js
https://react-select.com/
MIT License
27.57k stars 4.12k forks source link

What is the best way to publish this component? #40

Closed JedWatson closed 9 years ago

JedWatson commented 9 years ago

I'm creating this issue to roll together #26, #35 and #38 as a central place for feedback and ideas.

It appears there are several issues relating to the build process and use of JSX, including:

The simplest way to side-step these issues is to avoid JSX in this and any other packaged component, however I'd much prefer to just figure out how to solve this problem and have a great build and packaging process that works everywhere and doesn't have these issues.

In addition, I've created a common gulp tasks package (react-component-gulp-tasks) and a react component generator (generator-react-component) with the hopes of solving the problems of how to build and maintain react components for other developers too. It would be great to get some consistency in this space, imo.

To recap, my requirements for the build process / tasks are:

I would also prefer not to have to rename my javascript files to .jsx files. This is a purely personal preference; I'm willing to be pragmatic but ideally with the right pre-processor setup, you should be able to switch between using JSX or ES6 syntax in a file, and vanilla Javascript, without changing the extension.

I'd really appreciate everyone's input into how these requirements can be met, hopefully we can achieve all of them.

JedWatson commented 9 years ago

My current best idea (aside from dropping JSX) goes like this:

The downsides to this approach I can see are:

JedWatson commented 9 years ago

cc @julen @DavidWells @sebmck

Would also love to hear from @vjeux, @petehunt, @mjackson, @rpflorence, @zackify or anyone else who has experience with this.

KyleAMathews commented 9 years ago

I have a component-starter repo that has my setup for building examples pages + easy output of ES5 for distribution — https://github.com/KyleAMathews/react-component-starter

I use a Makefile to automate things but the same steps can easily be written with npm scripts or gulp/grunt.

One nice thing you can do if you're not worrying about Bower is include dist/ in your .gitignore file. It's nice to avoid compiled code in your git commits. If you do this note you'll need a .npmignore file as well that doesn't ignore dist/ as by default npm publish uses .gitignore unless there's a .npmignore file.

insin commented 9 years ago

I'm using a template project which does most of what's in your list; npm doesn't know about src/, git doesn't know about lib/, but has dist/ for a convenient version of the browser build.

Another thing I'm also doing where I'm using envify for development mode logging is putting envify in dependencies and using browserify config in package.json - I'm only currently doing this because React is also doing it, so you need to deal with envification when using the npm version (here's its package.json). - is there a nicer solution to that?

petehunt commented 9 years ago

Inline styles + whatever react-router does?

Sent from my iPhone

On Jan 3, 2015, at 11:06 PM, Kyle Mathews notifications@github.com wrote:

I have a component-starter repo that has my setup for building examples pages + easy output of ES5 for distribution — https://github.com/KyleAMathews/react-component-starter

I use a Makefile to automate things but this can easily be done as well with npm scripts or gulp/grunt.

One nice thing you can do if you're not worrying about Bower is include dist/ in your .gitignore file. It's nice to avoid compiled code in your git commits. If you do this note you'll need a .npmignore file as well that doesn't ignore dist/ as by default npm publish uses .gitignore unless there's a .npmignore file.

— Reply to this email directly or view it on GitHub.

julen commented 9 years ago
  • Point the main browserify and node.js entry points for the package at /lib

If you wanna be more generic with this so that the setup works for any libraries, you can have an index.js file as your entry point and have "main": "lib/index.js" in your package.json.

react-router does this for example, and since this module exports your library API it also offers a quick overview of the components available to developers.

  • An increase in generated code checked in to the repo, and more code duplication in general

I'd only track source files in git as @KyleAMathews says. You only perform the build step before uploading a new release to npm, but the resulting files don't need to be under source control.

This means there's no increase in generated code checked into the repo here (personally I wouldn't even track the browser bundles in dist/).

nmn commented 9 years ago

I do think a more complicated build process that finally produces a standalone UMD wrapped dist is the best option. It may have some upfront costs, but it has the best usability from a user point of view.

@petehunt inline-styles are more and more interesting to me. But we still need much better tooling to make things like hover states work with minimal effort. Having to use js for all of this stuff obviously makes things more complicated.

I still think a similar concept, where the CSS is still kept separate but the tooling still works the inline-styles way is the best way forward.

floatdrop commented 9 years ago

One big problem of packaging React component is how to manage styles:

Like @petehunt says, if you satisfied with inline styles - then you don't have those problems, but you will need to handle all :hover states for yourself. Downside of this - pseudo-elements (like :before and :after) can not be done this way.

There are couple packages to solve this: RCSS, react-style and others (just search for style in awesome-react). Often they injecting your styles in <style> tag and generate own selector for element (with :before). So it get things working.

But writing CSS in JS is not common in our team (because you have no autocomplete and syntax highlighting is bad).

@nmn can you look at react-styled - is this similar to what you want?

robertknight commented 9 years ago

I've started using a CSS-in-JS approach recently (using my own non-React specific lib ts-style) and from a developer's point of view, having the component logic, layout and styling all implemented in the same language is very convenient.

As Vjeux's presentation says, a lot of CSS pain points become trivial once the styling is implemented in JS. It also opens the door to using TypeScript and/or Flow to make CSS more maintainable. eg. If you're using ts-style with TypeScript you can easily determine whether a particular style is unused and remove it, or use IDE support (eg. with Visual Studio or CATS) for project-wide renaming of styles.

For wide adoption of CSS-in-JS I think there needs to be an officially supported React library for it though. For ts-style I try to make sure clean and readable CSS is generated to provide an easy migration path back to CSS if desired and support integration with tools like autoprefixer, but an officially supported styling library would be a better way to solve the problem @floatdrop describes that teams might be nervous about writing CSS in JS.

I'll also second @nmn 's point that pseudo-selectors are still very useful to quickly produce a lot of effects and having to implement those manually in React components would be a regression.

nmn commented 9 years ago

@floatdrop Hey, react-styled looks cool. But I'm looking for is basically exactly the opposite.

React-Styled, is trying to bridge CSS and JS. It gives you an old way to manage CSS with selectors etc, which only solves the performance issues not the modularity etc mentioned in the talk. I'm actually looking more towards Vjeux's presentation as well. I'm basically sold on the idea except for the pseudo selectors etc.

I think that the problem can be solved with a slightly different implementation. I've been thinking about it for a while and I think I have the final idea:

so in the presentation, the way to apply the styles is described as:

<div style={m(styles.container)} />

I'm saying let's change it to something like this:

<div data-styleid={idFor(m(styles.container))} />

Here the idFor function would be linked to a completely isolated separate (React?) Component that only renders style tags. It gets an object of styles. It then compares this to set of styles already rendered. If the same set of styles already exist it just returns a unique id for the styles. Otherwise, it creates a new style tag and returns a new id for it.

Something like this:

var styleTagsMap:[id:styles] = {...}
function idFor(styles){

  for(var id in styleTagsMap){
    if(deepEquals(styleTagsMap[id], styles){ return id };
  }

  var newId = generateNewUniqueID();
  styleTagsMap[newId] = styles;
  renderNewStyleTag(newId, styles);

  return newId;
}

Now since CSS isn't actually inline, it can be combined with pseudo selectors of all kinds. There would still need to be a solution regarding the syntax etc, but it's just a small leap away.

Also, since the CSS is completely id based it should have as good performance as the best practices in CSS land (BEM, OOCSS etc.). The basic concept is to flip the selector model. Instead of the CSS selecting elements, the elements are signing up for styles.

For further optimization:

DavidWells commented 9 years ago

Following the CSS in JS thread. There is also JSS https://github.com/jsstyles/jss . https://medium.com/@oleg008/the-important-parts-131dda7f6f6f seems like no shortage of solutions out there.

Trying to figure out the right approach seems to be the challenge.

This thread was about packaging react components for NPM though =)

In #38 I mentioned that the .js extension is not compiling correctly when rendering the component serverside in Node.

I'm stuck

error message:

/node_modules/react-select/src/Select.js:439
            return <div key={'option-' + op.value}
                   ^
SyntaxError: Unexpected token <
    at Module._compile (module.js:439:25)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/src/app/components/thiscomponent.jsx:3:11)
    at Module._compile (module.js:456:26)
    at Object.require.extensions.(anonymous function) [as .jsx] (/node_modules/node-jsx/index.js:26:12)
    at Module.load (module.js:356:32)

Thanks all!

sebmck commented 9 years ago

@DavidWells Because the raw JSX is being executed by node.

DavidWells commented 9 years ago

@sebmck so the component (which is an NPM package) needs to be converted to JS somehow by node? How would one do this?

Thanks

sebmck commented 9 years ago

@DavidWells You can't really until a build process is put into react-select or if you use something like node-jsx which has it's own set of issues.

DavidWells commented 9 years ago

I have node-jsx running in the project like:

require("node-jsx").install({extension: '.jsx', harmony: true});
sebmck commented 9 years ago

@DavidWells react-select uses the .js extension so you'll need to run require("node-jsx").install again with the .js extension.

DavidWells commented 9 years ago

requiring it twice doesn't work. Still a node runtime syntax error

In main node entry point server.js I tested:

require("node-jsx").install({extension: '.jsx', harmony: true});
require("node-jsx").install();

and

require("node-jsx").install({extension: '.jsx', harmony: true});
require("node-jsx").install({extension: '.js'});

no dice.

sebmck commented 9 years ago

@DavidWells Looks like it doesn't allow you to run it more than once. See here.

DavidWells commented 9 years ago

@sebmck so an 'isomorphic' codebase needs to be all .JSX or all .JS? That seems weird.

sebmck commented 9 years ago

@DavidWells Hence the reason for this issue.

DavidWells commented 9 years ago

@sebmck shizzz. Okay thanks for the input.

My work around right now is hardcoding the react components into the app (outside of the node_modules folder) but obviously this is less than ideal =)

STRML commented 9 years ago

I've been doing more or less the same as @JedWatson. /src is built to /build on commit, and a pre-commit hook with a simple Makefile makes sure it doesn't get out of sync.

Yeah it's not exactly perfect, but it is very nice to not have to include an entire runtime to run ES6 or JSX code. It's not a whole lot different than writing a lib in CoffeeScript and building it for NPM.

tomchambers2 commented 9 years ago

I'm having the same issue as @DavidWells - using node-jsx on server side and choking on the .js files. For now I've just changed the module filenames from .js to .jsx.

JedWatson commented 9 years ago

Thanks for the feedback everyone.

I've updated the build process so /src is now built to /lib by 6to5, which will work in node and in browserify / webpack without further transformation. /dist is still built by browserify for people who would prefer to have a drop-in-ready version of the script. I'm writing up my learnings and will post them here when they're published.

eedrah commented 8 years ago

Hi @JedWatson - you mentioned writing up your learnings. Any thoughts not already expressed in this thread?

Also, did a consensus come of the css discussion and/or is there a more appropriate place to talk about that?