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

[RFC] [WIP] Tool registry #197

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

[RFC] [WIP] Tool registry #197

wants to merge 8 commits into from

Conversation

fkling
Copy link
Owner

@fkling fkling commented Feb 20, 2017

Note: The code is pretty raw and will change a lot. I'm primarily looking for feedback/questions on the idea, but I also appreciate feedback on parts of the code.

Problems

There are a couple of problems with the current way of how astexplorer makes different parsers and transformers available:

  • Website and parsers/transformers are all (split) bundled together. That means that upgrading one parser could affect other parts of the website or other parsers.
  • Providing the latest version of a parser requires manual intervention (rebuild and push website).
  • Only one version of a parser is available (except where we workaround it by (manually) providing multiple packages). A snippet will always use the latest available version of a parser, no matter with which version it was created.
  • Confusing relationship between transforms and parsers; the parser is just used for displaying the AST, it is not used by the transform itself. Therefore any configuration adjustments for the parser don't have any effect on the transforms.

This proposal would solve all of these issues.

Proposed solution: A tool registry and unified interface

screen shot 2017-02-19 at 11 15 10 pm

The registry is basically a separate endpoint that builds parsers (more generally: tools) on demand, independently of the website itself and other tools. For example, /api/v1/tools/acorn/4.0.11 would return version 4.0.11 of acorn.

The tools are stored in registry/tools/<toolname>/ and pretty much use the same interface as the current parsers/transformers in website/src/parsers/. They are still associated with a category (i.e. JavaScript, CSS) but not grouped by it anymore (only in the UI).

In addition to the wrapper/interface files, package.js contains meta and build information for a tool, for example:

const webpackConfig = require('../../webpack.default.conf');

module.exports = {
  name: 'acorn',
  displayName: 'acorn',
  homepage: 'https://github.com/ternjs/acorn',
  category: 'JavaScript',
  versions: [
    {
      main: './v2.js',
      dependencies: {
        acorn: '^2.0.0',
        'acorn-jsx': '^2.0.0'
      },
      webpackConfig,
    },
    {
      main: './v3.js',
      dependencies: {
        acorn: '^3.0.0',
        'acorn-jsx': '^3.0.0'
      },
      webpackConfig,
    },
    {
      main: './v4.js',
      dependencies: {
        acorn: '^4.0.0',
        'acorn-jsx': '^3.0.0'
      },
      webpackConfig,
    },
  ],
};

The field versions contains information about which versions are available (taken from the semver range in the dependencies entry), which entry file to use for a specific version, and which build configuration (if adjustments are needed) to use.

Given a specific version, the build script finds the entry that satisfies the requested version, generates a package.json file and copies the source files into a temporary folder. In there it installs the specified dependencies, the requested tool version and runs webpack. Webpack will generate an AMD bundle.

Unified interface

One big difference is that transformers also have to provide a parse() method (instead of specifying the ID of a parser to use). This method is supposed to delegate to the parser the transform is using. That means, whether a tool is a transformer only depends on whether or not it implements a transform method.
Overall this should simplify the store logic and other places on the website.
Another positive side effect of this: The "parser settings" logic can now be reused for transformer settings.

Building versions on demand

If a bundle isn't available for the requested version, the webserver creates a new build job. The job is processed by another process. On my machine, building acorn took ~2s, building jscodeshift took ~13s. I'll have to see how the build times are on the server.

Automatically build the latest version

A cron job can run to periodically check all tools for new available versions and build the latest version automatically.

Automatically build inventory

Another cron job will take the information of all the package.js files and generate an inventory that lists all the available tools and versions. This information is used on the website to let the visitor choose a tool (see previous screenshot).

More ideas

  • Be able to "bookmark" favorite tools so that they can be selected from the menu more quickly.
  • Show a banner if a snippet uses an older version of a tool.

Current limitations of the solution

  • At the moment the build script expects all tools to be published to npm. More thought has to be put into how package.js should be structured for non-npm tools.

ToDo

  • Tool building/loading error handling
  • "Port" all parsers and transformers
    • JavaScript
    • CSS
    • GraphQL
    • Handlebars
    • SQL
    • WebIDL
    • YAML
  • Create gist snippets v2 format
  • Be able to load Parse and gist v1 snippets
  • Be able to bookmark tools for quick access
  • Refactor/redesign version information (top right corner)
    ... and probably more

@RReverser
Copy link
Collaborator

I was recently thinking about this problem too, but instead of custom tool builder I rather thought about using wzrd.in which will do bundling & caching for us.

@skratchdot
Copy link
Contributor

This is looking good to me.

I wanted to take a peek locally (to preview the changes), but I keep seeing "build timeouts". I'm definitely not doing something right. I'll try to take a look at the registry branch again in a few days because it's probably something simple that I'm missing.

@fkling
Copy link
Owner Author

fkling commented Feb 21, 2017

@RReverser: Maybe wzrd.in can be used for dependencies required from transformers, on the fly. I don't think it would give us enough flexibilities for the existing parsers and transforms. Just look the special treatments we have to do for the some of the parsers/transforms in the webpack config.

@skratchdot:  Thanks! Sorry, I didn't really provide any instructions for how to run it. You need to have redis installed. Then the following commands should do it (in three different windows)

cd website/
yarn
yarn watch
cd server/
yarn
yarn start
cd registry/
yarn
yarn start-worker

@skratchdot
Copy link
Contributor

skratchdot commented Feb 21, 2017

@fkling - That's basically what I tried, although this time, I'm getting slightly different errors.

I know at one point I had to run mkdir bundles and I also tried BUNDLE_DIR=../bundles node generate-inventory.js.

In my current setup, I'll run the commands you pasted. Everything starts without error.

I hit http://localhost:8080/

Then in my server console I see:

Serving tools..
Serving static files...
Server listening on port 8080!
asd
info: 2017-02-21 04:10:51 Requesting build of recast@latest...
error: 2017-02-21 04:10:53 Error: Unable to build package recast@latest

And in my registry console I see:

Error: recast doesn't exist.
    at buildVersion (/Users/[username]/Projects/git/astexplorer/registry/build.js:32:27)
    at Object.<anonymous> (/Users/[username]/Projects/git/astexplorer/registry/build.js:22:1)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.runMain (module.js:605:10)
    at run (bootstrap_node.js:420:7)
    at startup (bootstrap_node.js:139:9)
error: 2017-02-21 04:12:24 Error: Unable to build package recast@latest

I've tried yarn adding missing modules at different times but currently, I've tried yarn add recast in the registry folder, and I still see the error above. At one point I was seeing cssdom errors (not sure how I got in that state)- but since then I've started clean, and followed your instructions, and am seeing the error above. Any ideas?

@fkling
Copy link
Owner Author

fkling commented Feb 21, 2017

Oh I see. You can only build what is available in the registry/tools/ directory. That's currently acorn, babylon and jscodeshift (and also only the versions that match the ranges in the package.js files).

@skratchdot
Copy link
Contributor

I just didn't understand why hitting localhost:8080 automatically tried to build recast. I guess I need to modify the code in website to get things to work? I'll trace through it when I have a bit of time. Thanks for the help (and the work)!

@fkling
Copy link
Owner Author

fkling commented Feb 21, 2017

Oh, that's probably because recast is picked up from local storage. Deleting the storage key should work.

As I said, it's very much WIP 😉

@skratchdot
Copy link
Contributor

Nice!! That fixed the recast issue I was seeing. Thanks!

I had to make 1 small change b/c I was running into a parser is null error, but I've got things rendering now.

I changed:

export const showTransformer = createSelector(
  [getParser],
  (parser) => !!parser.transform
);

To:

export const showTransformer = createSelector(
  [getParser],
  (parser) => !!(parser || {}).transform
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants