Huemul / chimi

Validate JavaScript code from your README
https://github.com/Huemul/chimi
33 stars 0 forks source link

Design Ideas #10

Closed franleplant closed 6 years ago

franleplant commented 7 years ago

Hi Guys! First of all: Good Job on pulling this out!

It's great to have a semi functional version, but I'd like to bring to the table a sane discussion about the scope of this project and what would be its responsibilities, its extension points and what should definitively not do.

Objective

In a broad sense this library should allow an easy way to run javascript snippets of code inside markdown files.

The snippets tend to be part of the code documentation and the idea is to help keep those pieces of code alive in the sense that we validate that they can pass static and dynamic tests.

What should the lib do?

In here we definitively want to extract code inside Javascript and potentially Typescript from markdown files and provide a way for the lib user to run those files. It's important that we allow the user to inject processors to the code pipeline as an extension point. See more below.

I think that we should really define this libs responsibilities because the less we can do to accomplish this task the better, because we would be kipping it simple while providing extension points (see extension points section)

What should the lib do not?

The lib should interfere as little as possible with the markdown documents, ideally, this lib should be able to work on any existing project without editing their markdown files. This comes in reference to all the DSL you guys created regarding dependencies (more on that later), environments, et al.

First of all, using '''js, flag1, flag2 is going to keep the files being highlighted as Javascript by Github and other markdown displayers? If the answer is no, which I suspect it is, then this type of dsl should be completely avoided. This would probably break with your env DSL, which I would probably suggest that we provide a single entry point for a project with all the environment config, something like snipperrc, and deal with those global stuff there, and not allow it to be set on snippet basis.

As for templates, I'm not to in love with that. I would definitively leave that till later, or to be done through an extension point. I don't think that this lib should aim to be a completely testing lib such as Karma or Mocha, but just a code extracting and running tool. Another good example of this is that if we want snippets to have testing capabilities, i.e. assert, test suites, etc, we should definitively not provide these, we should instead let the use require those as regular modules (this is of course a critic to mocha, jasmine, jest that use all global stuff, in comparison with tape, ava, et al which work as regular modules ). Remember that most of the time these snippets would be simple examples of code and not full fledged pieces of code, we should leave that to testing frameworks.

Dependencies should be treated as regular dependencies, don't add a dsl for this, we already have several syntaxes to define module dependencies, use those i.e. CommonJS, ES6, AMD, etc. I strongly favor CommonJs or ES6.

A more thorough lib would probably bring context into all the snippets, i.e. if a snippet is above a function then that function would be implicitly in the snippet environment, but I don't think we should go that far right now, because that would require us to use parsing tools and deal with ASTs instead of just doing dumb text/regexp manipulation. But we could have this as goal for version 2 !

Extension points

This is one of the toughest and most important parts of any lib.

The biggest thing for me is to provide a way to hijack the code processing pipeline in order for the users to insert their own processes to the pipeline, i.e. babel, typescript, flow and any static analysis tools such as linters.

We have some alternatives here, but the less we do here, the better, and what I mean with this is that we should provide minimum opinions, and maximum versatility.

One way to achieve this is to provide middleware-like extension points like express or like browserify/webpack plugins/loaders, and let those extension points do whatever they like.

Alternatively we could support any rc file which would be cumbersome and difficult to adapt to new things.

Another good alternative here is to simply output all snippets into disc, i.e. tmpSnippets/myFile.js and then let the user do whatever processing they want over those files, this has the benefit of having maximum extensibility and the low points of being potentially slow and hard to set up (think webpack config for your snippets, but, users could probably hook that to their already existing build systems). This alternative should also provide an easy way of running all snippet files, something like a index.js that calls all the snippets files and thus making them run. Another good benefit of this alternative is that we probably wont need to worry about the code's env, because the user controls the way of running it, so if she needs jsdom the let her set it up, if it needs something globally available then let her set it up (in a regular js file)

I personally like, for this first iteration, the latter option, dumping all into disc provides maximum extensibility with minimal effort from our part.

TODO: create an example of using babel

Let me know what you guys think, I can probably help you with a bunch of stuff

Bonus

With all this design in mind you don't need to worry about things such as throw because the user has the power. And also, don't think this as a testing framework, and that's way you don't need to support skip, you can let the user comment the code! Or dont assert anything

fvictorio commented 7 years ago

Hey, thanks a lot for the thorough analysis.

I agree that the DSL for configuration should not be mandatory: you should be able to do everything in a centralized configuration file without having to modify the markdown file. That being said, I do like the idea of having inline configuration, mainly to allow the user to ignore snippets. The way to do that is something to discuss.

Extension points are something that we didn't talk about, and it's true that it's very important. Thanks for bringing that up. Right now, the (wrapped) snippets are being executed by an external process (namely node, but it could be anything), so a simple extension point would be to allow the user to provide a function to wrap-up the snippets and a command to run them. What do you think about that?

I'm not sure about writing the snippets to disk. I'd like the user to be able to run all their snippets after some simple configuration. We could entertain writing them to stdout, maybe, but that would mean having to add a separator or some ugly thing like that.

Transformations are another important issue. I don't know how to approach that right now. Maybe some pipeline of external commands?

franleplant commented 7 years ago

The wrapper function customization makes sense, but if you treat everything as a module then there's less things you need to worry about, because you just go standard.

Special snippet metadata should be placed inside the snippet as code comments of the form //@ignore or similars. This gives you some sort of dsl but removed anything from the markdown itself.

The extention point could be done in multiple ways, but it can get really complex easily, see the horrible webpack loaders as an example. I would say that the most pragmatic and easy way is to output to disc. We could discuss further about the format but something super obvious would be:

Where snippetN.js should contain a single snippet. This will make each snippet an es6 or common.js module without any effort.

And where index.js will be a helper file that inside requires or imports all the snippetN.js files.

So, in order to run all snippets the user will just do node snippets/index.js

I like this approach because it's super dumb and super versatile. You are supporting virtually any extention points you may think off: babel compilation, flow, eslint, etc, and without creating APIs that would need you to carefully design and support them.

There are some drawbacks of course, namely performance and the limitations of modules, where you wont be able to customize the environment, but of the top of my mind, you could always mess up with the global or window object in order to provide easy access to say, the code in your library.

On Wed, Apr 5, 2017, 21:07 Franco Victorio notifications@github.com wrote:

Hey, thanks a lot for the thorough analysis.

I agree that the DSL for configuration should not be mandatory: you should be able to do everything in a centralized configuration file without having to modify the markdown file. That being said, I do like the idea of having inline configuration, mainly to allow the user to ignore snippets. The way to do that is something to discuss.

Extension points are something that we didn't talk about, and it's true that it's very important. Thanks for bringing that up. Right now, the (wrapped) snippets are being executed by an external process (namely node, but it could be anything), so a simple extension point would be to allow the user to provide a function to wrap-up the snippets and a command to run them. What do you think about that?

I'm not sure about writing the snippets to disk. I'd like the user to be able to run all their snippets after some simple configuration. We could entertain writing them to stdout, maybe, but that would mean having to add a separator or some ugly thing like that.

Transformations are another important issue. I don't know how to approach that right now. Maybe some pipeline of external commands?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gillchristian/snipperjs/issues/10#issuecomment-292032162, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxEJwbO2MqnpghL1_av5NY_TBwC6sZCks5rtCyxgaJpZM4M07Hw .

gillchristian commented 7 years ago

Thanks @franleplant for giving such in depth analysis/feedback.

In a broad sense this library should allow an easy way to run javascript snippets of code inside markdown files.

The snippets tend to be part of the code documentation and the idea is to help keep those pieces of code alive in the sense that we validate that they can pass static and dynamic tests.

I totally agree with that, so let me first explain a bit how the snippets are ran, that might also answer some of the design decisions.

It starts by grabbing all the code snippets in a MD files tagged with js or javascript, and for each one injects the dependencies listed in the config (.snipperrc) as CommonJS requires, creates a child process (running node on it), pipes the snippet and captures stdout & stderr to check if all went well. And after every snippet ran we show that stdout & stderr alongside a summary.

Some things to point out:

ideally, this lib should be able to work on any existing project without editing their markdown files.

We already have a .snipperrc for global configuration. And hope that it will be enough for most cases. For now just a few things are configurable there (timeout, the MD file and the aforementioned global dependencies). I'd like to add other options here: .snipperrc.js, .snipperrc.json or event inside the package.json.

First of all, using '''js, flag1, flag2 is going to keep the files being highlighted as Javascript by Github and other markdown displayers?

The first line of the code snippet (the one with the extension tag) does not get displayed in Github regardless of what you add to it. I also checked some other online MD editors too, and I guess Bitbucket and Gitlab do the same.

I don't think that this lib should aim to be a completely testing lib.

Our idea is to just let the user run the snippets to check if everything is ok. Which means the code runs and does not throw. That's why we show a summary at the end with the stderr of the failing cases. That's the only "assertion" the lib does.

A more thorough lib would probably bring context into all the snippets, i.e. if a snippet is above a function then that function would be implicitly in the snippet environment

Our thought on that was to let the user group snippets and run them all together as one.

Extension points I really love that.

On how to do it I agree with @fvictorio here, I dislike the option to dump to disk.

provide middleware-like extension points like express or like browserify/webpack plugins/loaders, and let those extension points do whatever they like.

I think this would be easy to implement since we now have a pipe like flow. We could even have pre and post run hooks.

Regarding the DSL config. The format is of course open to discussion, I'll add you @franleplant to the repo and we can continue there. Thing is I got really exited with this repo/tutorial and went for that LISP like syntax.

We went (actually is not implemented yet haha) with an optional per snippet config to cover those special cases when you want to have one snippet that is different to the rest.

Of course all of that should be configured on the config file and used sparingly as a per snippet basis when an escape hatch is needed.

gillchristian commented 7 years ago

Special snippet metadata should be placed inside the snippet as code comments of the form //@ignore or similars. This gives you some sort of dsl but removed anything from the markdown itself.

The think I like about having the metadata hidden is that it keeps the snippet simple for the user reading it. That's why I also want to list dependencies there. Of course you can do it with JS inside the snippet, that would actually work, be will make the snippet bigger.

```js # (no-globals) (deps (_:lodash)) (This won't be seend by the end user)

_.map( ... )

vs

//@no-globals
const _ = require('lodash')
_.map( ... )

I know a DSL might be an overkill but hey I got exited about doing a perser. But again it can be discussed, might be completely another thing. Maybe even just a JS object:

```js { noGlobals: true, dependencies: { _: 'lodash' } }

franleplant commented 7 years ago

I've tried the the dsl and github keeps highlighting correctly so I have no problems with that. Example: https://gist.github.com/franleplant/43c42f02e32893709b00c93e17b144de.

I dislike the idea of creating a custom pipeline mostly because it would make it harder for the user to interact with the code. Have in mind that setting up build systems in modern apps tends to be a complex thing, and most of us tend to make it declaratively instead of programmatically.

This also comes coupled with the fact that we are the ones that are running the code. Which I think is too much responsibility for this lib.

A very simplistic version of this lib that fulfills the objective that I wrote and provides maximum extensibility and control by the user while making our lives, as lib authors, much better, is the idea of the lib outputting all snippets as js files, modules, in disk, and then the user can run anyway she likes the files and also can process the code whatever she likes, and probably will make it easier for the user to use her already existing build system (think of adding the snippet temp directory to their build system input).

I can even go further and tell you that with this sort of simplistic approach we would be letting the user have maximum control of the execution of the code, we could even let them use testing frameworks on top of the files we output. Think mocha, jest, or whatever, effectively solving the env problem, because those frameworks already work with that. This of course applies to simplistic runners such as the snippets/index.js, which just runs all the modules one by one, and which could support custom environments, without any APIs from our part.

The thing that I want to emphasize is this:

We need to have in mind that creating a nice middleware-style extension point will take time, will require a careful API design, will require documentation, will require that the users manually and programmatically set up build systems contrary to what we tend to use: declarative files, i.e. webpack.config.js, .babelrc, tsconfig.json, etc. It's a hard problem which has lots of edge cases. Think about all the processes that we run on our code in modern js developments!

An example of this is the Webpack loaders which use poor syntax, they are very hard to debug, and very hard to reason about, at least at first. Browserify transforms are a little better but they tend to be complex just because of the nature of the problem domain.

franleplant commented 7 years ago

Example: Using the lib with the disc way

# Simple running
snipperjs readme.md && node ./snippets/index.js
# Testing frameworks run
snipperjs readme.md &&  tape ./snippets/*.js
# Custom env with simplistic running
snipperjs readme.md && node myCustomEntryPoint.js

# Testing, linting, flow
# flow and eslint APIs are idealized here
snipperjs readme.md && flow ./snippets && eslint ./snippets && node ./snippets/index.js

The more I think about it the more I like this API, it plays well with anything

gillchristian commented 7 years ago

I am starting to like the dump-to-disk approach you propose.

There is one issues I see, maybe just 'cuz I was already sold with the current approach.

If each snippets means a module and the user just wants to run the code to assert that all is ok, in the entry point, would she be responsible for importing each file? It is even more complicated when the user has more than one MD file. I see that as too much overhead.

So maybe we could provide some kind of runner that does that

# dump to disk
snipperjs generate --file readme.md --output ./snippets
# simple running
snipper run  --entry ./snippets/index.js

# testing framework
tape ./snippets/*.js
# custom run by user
node myCustomEntryPoint.js
# linting, flow
flow ./snippets && eslint ./snippets

I like that because we cover a very broad range of cases. On one side the user who just want to know that snippets are fine, does not even want to have an entry point, on the other side the power user who wants to use a lot of tooling.

With @fvictorio we had in mind some other stuff. Like an interactive mode for books or tutorials. That was way in the future anyways, like v5 or something :stuck_out_tongue:

franleplant commented 7 years ago

Thats why I brought the concept of snippets/index.js.

That file would be autogenerated by the lib and would only contain import statements for every snippets, so running all snippets is easier.

I like the idea of interactive mode! And I agree should be dealt with later.

I think that with the code that we alreadt have, implementing dump to disk should be easy, at least getting a prototype of it.

I might have some time to do it over the weekend, lets coordinate so we dont step on each other feets. I will call out to you guys when I start working on it, if at all.

On Thu, Apr 6, 2017, 22:35 Christian Gill notifications@github.com wrote:

I am starting to like the dump-to-disk approach you propose.

  • The entry file solves many of the context problems.
  • It is really really easy to use other tools besides running the code (flow, eslint, etc).
  • We don't even have to solve the hard problem of providing hooks into the pipeline.

There is one issues I see, maybe just 'cuz I was already sold with the current approach.

If each snippets means a module and the user just wants to run the code to assert that all is ok, in the entry point, would she be responsible for importing each file? It is even more complicated when the user has more than one MD file. I see that as too much overhead.

So maybe we could provide some kind of runner that does that

dump to disk

snipperjs generate --file readme.md --output ./snippets# simple running snipper run --entry ./snippets/index.js

testing framework

tape ./snippets/*.js# custom run by user node myCustomEntryPoint.js# linting, flow flow ./snippets && eslint ./snippets

I like that because we cover a very broad range of cases. On one side the user who just want to know that snippets are fine, does not even want to have an entry point, on the other side the power user who wants to use a lot of tooling.

With @fvictorio https://github.com/fvictorio we had in mind some other stuff. Like an interactive mode for books or tutorials. That was way in the future anyways, like v5 or something 😛

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gillchristian/snipperjs/issues/10#issuecomment-292392380, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxEJ0XbxakwLxbYjSDiJ6k3e9bV0-xLks5rtZLwgaJpZM4M07Hw .

gillchristian commented 7 years ago

That makes sense.

A very simple first version seems reasonable. After that we can talk about a runner a some more fancy stuff.

gillchristian commented 7 years ago

I've been talking with @fvictorio about the initial idea we had about being able to just npm install and just run it. And we don't think it would be a problem to have the responsibility to run the code as long as it is well defined. But on the other hand there are many benefits on dumping the snippets to disk. So we think splitting in separated packages would be a good idea.