expressjs / discussions

Public discussions for the Express.js organization
63 stars 15 forks source link

Browser-support for jshttp (and maybe other) modules #10

Open jonathanong opened 8 years ago

jonathanong commented 8 years ago

from https://github.com/jshttp/content-type/pull/7#issuecomment-200635582 and from the TC meeting w/ @wesleytodd

my thoughts:

my questions:

going forward, i'd like to build a sample repo for standardizing all modules including (most of which is already pretty standardized thanks to @dougwilson):

if we all agree on it, i don't mind making the PRs for a lot of the modules to make them all consistent

dougwilson commented 8 years ago

The #1 thing I really don't want is any type of built asset being checked into the code branch, because it is far, far too easy to forget to rebuilt before making every commit, ending up with noise commits rebuilding (or forcing the rewrite of git history) or even making releases without committing a new rebuild, resulting in bad releases.

I don't really develop for web browsers, but from what I've seen, since there are so many different requirements, simply having the users build the modules according to their own needs seems like the best option, but IDK.

Pretty much the only users I've ever seen asking for UMD files committed to the repo (which is the main issue here) is those trying to use bower to depend on the modules.

blakeembrey commented 8 years ago

I can't answer these properly as I'm not the intended audience, just someone who's also gone through the same steps. GitHub releases seemed the best of bad options for me, since most of the time it's Bower users requesting the feature so there's no alternative (other than checking code in).

On the dependency note, I think it becomes required to bundle them unless we get a more complex bundle step built, right? Otherwise dependencies need to be written in a way that they can be imported from AMD, CommonJS and global. That's where supporting UMD is the hardest for me, it's simpler when there's no dependencies involved and I'd love to hear someone's solution here (I haven't seen anything).

On CDN support, it might make sense if it can be automated but it seems the only way to do that is checking in built files :frowning: Maybe we could get release assets made a part of this? https://github.com/cdnjs/cdnjs/blob/master/documents/autoupdate.md

Aside from that, a model repo makes sense - whether it's a template or a collaborative pass at a simpler repo doesn't bother me.

An important thing you've noted about babel is (I assume why you mention it) older browser support for new JavaScript features. We should probably have a baseline willing to support, but I feel like it would align closely with our Node.js baseline too.

jonathanong commented 8 years ago

just tried https://github.com/defunctzombie/zuul and it seems pretty straight forward. seems like travis + saucelabs integration is pretty straightforward, but requires a lot of configs.

try it out:

git clone git@github.com:jshttp/basic-auth.git
cd basic-auth
npm i
npm i -g zuul
zuul --local 4000 --ui mocha-bdd -- test/basic-auth.js 
# open http://localhost:4000/__zuul

result:

screen shot 2016-03-23 at 11 53 02 pm screen shot 2016-03-23 at 11 53 17 pm

i couldn't see the coverage report per file. not sure if that's a bug or zuul just doesn't support it.

@defunctzombie @vvo any thoughts?

vvo commented 8 years ago

My two cents:

Publishing packages for Node.js & the browser(s)

so basically in npm you would have:

One other possible and "next-gen" solution would be to use rollup.js and instead of getting a full file tree, you woul only have one UMD build for everyone.

Relevant discussions:

Testing

As far as I can tell, your library do not really rely on any specific browser feature, I would not even bother testing this in multiple browsers.

Indeed, there are a lot more chances your multiple browser test will fail because of the testing infrastructure than because of the actual code paths/flow/bug of yours.

What we do at https://github.com/algolia/ is:

Trying to make all unit tests pass in all browsers is, in the end, a bit of a time waste I do find. I have had more bugs with zuul/karma/saucelabs than bugs in my actual code.

Selenium is currently the only robust solution to start and control browsers. Saucelabs is selenium based.

As for test launchers, I have been a good contributor to zuul but still think Karma is a better solution. Karma has a more stable ecosystem and lots of contributors. Zuul is nice but still a bit hacky.

As for test coverage I would go fo another solution than zuul, test coverage should in most cases not be browser dependent.

Sorry if all of this is out of topic, that's mostly my whole experience with testing so far :)

wesleytodd commented 8 years ago

Wow, I go to sleep and it looks like we have more people who care and have opinions on this than I thought!!

1. UMD

I would actually argue that we don't need UMD. We are talking about ADDING support, and so we get to decide how we want to support consumers. CommonJS is both how we are authoring now, and supported by the main front-runners for browser build tooling (Browserify and Webpack).

The problem we all face when choosing what to support with tooling is that the landscape changes too fast. So In my opinion we just need to publish the one format that is most popular and other people can write shims or build steps around it if they want to use less popular tooling.

2. Publishing Dist

The easiest long term IMO is hosting builds on js.org or the main express website and linking to it in the readme. This would obviously need to be a part of the release procedure and would probably be nice if there was a common system all the packages could use.

Full dependency bundles are a weird topic. In my opinion anyone who uses them is actually doing themselves a dis-service. So providing them in my mind is kind of like promoting a poor practice. BUT, if the person is a newbie and just trying to get their work done, then it is fine. Also, FWIW, most of the packages I see as candidates for browser support would not be newbie oriented packages.

3. Babel/ES2015

There are so many reasons NOT to do this yet. First and foremost Babel is young as tools go and still in flux. Over the past few months I have integrated Babel into most of the projects I work on. It has been the biggest time suck and annoyance that I have experienced with javascript tooling to date. Just including it in your package.json results in ~double the install time. Also, the benefit it gives is minimal at best. That is not to say I don't LOVE the new ES features. But until the tooling matures I don't think this is something we should take on.

Also to hit on the browser support issue, IMO if we have to make changes or include polyfills for specific browsers it is fine to just say no. So including babel just to get browser support on one IE version back seems silly. You can just tell the person in the docs they need a certain pollyfill to get support for whatever they need.

4. Other Package Managers

No. :)

5. Testing

I am fine with ANY solution that works for people. But the requirements I think we should set out are as follows:

  1. Must not be fickle.
  2. Must allow us to easily integrate with existing travis ci
  3. Must be easy to add and remove browsers as the support needs change
  4. Should be easy to document which modules support which browsers
  5. Should be minimal work to maintain

Whatever solution we pick would be awesome to document in an example module, and possibly build a yeoman generator for. If we start with a sample repo I can easily convert that into an actual generator that we can use when setting up new projects or run against old projects to setup certain parts of the new style.

In my experience, I have had success with mocha and sauce labs. The tests I run for nighthawk have been stable but are admittedly limited. I don't think there is a problem with slowly building out the list of "browser supported modules" and dealing with the problems we encounter on a per-repo basis. But I do think that choosing some common tool set is a good idea.

6. Browser specific builds (aka package.json browser field)

The main goal for browser specific builds would be to minimize the code footprint to whatever is useful in browsers. We wouldn't want to expose different functionality, but if a feature makes sense to work differently in the browser it would be nice to have it work. One example of this MIGHT be the cookies module. I think it would make sense for that module to be able to be used as a FE middleware and just set/delete/get browser cookies directly with the DOM api.

Finally

I think the main take away for me is this:

"Browser support" means many things to different people, and sometimes that means ALOT more work. In those cases, I would prefer to say, "Lets just not..."

If this is the mentality we adopt then there are basically 3 steps to getting "browser support":

  1. Choose which modules are candidates for browser support
  2. Get existing test running in some set of browsers (TBD based on the module in question) via CI
  3. Update readme's

If we think it is important to put more work into it then the steps are probably much harder...

wesleytodd commented 8 years ago

https://github.com/pillarjs/path-to-regexp/pull/74

^^ That is the first and simplest way to get things running in browsers. Feedback?

jonathanong commented 8 years ago

fwiw i'm okay with only supporting phantomjs until we get real browser testing working. saucelabs has been only a pain for me, but i don't know any alternatives.