Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
JedWatson opened this issue Jan 4, 2015 · 25 comments
Closed

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

JedWatson opened this issue Jan 4, 2015 · 25 comments

Comments

@JedWatson
Copy link
Owner

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:

  • Hopefully, ability to use JSX
  • Efficient development with on the fly regeneration of scripts (currently implemented via Watchify)
  • Live-reload of scripts and css for the examples
  • Compatibility with Browserify and Webpack build processes
  • Support for server-side rendering with Node.js
  • Generation of standalone package for Bower (is this actually useful?)
  • Easy ability to publish to npm / gh-pages

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
Copy link
Owner Author

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

  • Place the source in a /src folder, anything in here can have JSX / ES6 in it
  • Build from /src to /lib, probably with 6to5 (which includes a JSX processer)
  • Point the main browserify and node.js entry points for the package at /lib
  • Build from /lib to /dist with Browserify for the standalone distribution

The downsides to this approach I can see are:

  • An increase in generated code checked in to the repo, and more code duplication in general
  • An even more complicated build process
  • In node.js packages /lib is generally used for the "source" code of the package... in this case it would be generated, which may be confusing. Maybe we can come up with better folder names.

@JedWatson
Copy link
Owner Author

cc @julen @DavidWells @sebmck

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

@KyleAMathews
Copy link

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
Copy link

insin commented Jan 4, 2015

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
Copy link

petehunt commented Jan 4, 2015

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
Copy link
Contributor

julen commented Jan 4, 2015

  • 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
Copy link

nmn commented Jan 10, 2015

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
Copy link

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

  • Making styles extendable
  • Solving CSS specificity problem
  • Escaping global scope
  • ... all other problems from React CSS in JS.

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
Copy link

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
Copy link

nmn commented Jan 10, 2015

@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:

  • Server side rendering will be trivial
  • When dealing with a large app, there can be a solution where common styles can be put in style tags before they are actually needed server-side to improve the performance further and make for a CSS file that can be cached.
  • Some optimizations can be made such that if two elements have almost the same styles, they can be given the same id, with the differences applied as inline styles

@DavidWells
Copy link

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.

  • Anyone have a reason why that is?
  • Is there a flag I need to pass for .js files to be treated as JSX?
  • Or because I'm not using reactify?

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
Copy link

sebmck commented Jan 11, 2015

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

@DavidWells
Copy link

@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
Copy link

sebmck commented Jan 11, 2015

@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
Copy link

I have node-jsx running in the project like:

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

@sebmck
Copy link

sebmck commented Jan 11, 2015

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

@DavidWells
Copy link

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
Copy link

sebmck commented Jan 11, 2015

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

@DavidWells
Copy link

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

@sebmck
Copy link

sebmck commented Jan 11, 2015

@DavidWells Hence the reason for this issue.

@DavidWells
Copy link

@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
Copy link

STRML commented Jan 11, 2015

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
Copy link

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
Copy link
Owner Author

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
Copy link

eedrah commented Apr 5, 2016

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?

bkoltai pushed a commit to bkoltai/react-select that referenced this issue Sep 20, 2017
Update to JedWatson/react-select v1.0.0-rc.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests