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

Proposal: Make the project generalized #57

Closed
Naddiseo opened this issue Sep 5, 2014 · 9 comments
Closed

Proposal: Make the project generalized #57

Naddiseo opened this issue Sep 5, 2014 · 9 comments
Assignees
Milestone

Comments

@Naddiseo
Copy link

Naddiseo commented Sep 5, 2014

Currently, this project is geared towards ES, but since the interface is so nice for building asts, I'd like to use it in other parsing projects.
Would it be possible to make the project more generalized so that it doesn't load all the types in def/* unless requested?

@benjamn
Copy link
Owner

benjamn commented Sep 5, 2014

@Naddiseo That's a great point. I've been thinking there would be a lot of value in allowing users of the library to instantiate their own copy of the type system, so that when (for example) Recast adds the custom File type (see https://github.com/benjamn/recast/blob/master/lib/types.js), that doesn't interfere with any other users of the ast-types library.

Something like this?

var types = require("ast-types").fork(/* pass true to start fresh? */);
types.registerDefs("ast-types/def/core.js");
types.registerDefs("./custom/defs");
types.registerDefs("./more/defs");

@Naddiseo
Copy link
Author

Naddiseo commented Sep 5, 2014

That would work.
I was also thinking something like providing a language or something.

If the files under /def were moved to say /langs/ecma
Maybe something like:

var types = require('ast-types');
/*
languages.ECMA is defined as an array containing what is currently in `def`
*/
var langs = types.languages; 
types.registerDefs(langs.ECMA)

This would allow ast-types to possible build up defs of other language to aid other parser builders (or a separate repository for language defs).

Some other things that need to be thought about

  • I think the builder expects there to be at least def("Node") / def("SourceLocation") / def("Position") already defined. So they might need to be predefined and loaded.
  • It might be useful to expose /lib/shared

@Naddiseo
Copy link
Author

Naddiseo commented Sep 8, 2014

Another thing that I've found using the builders is that I want to be able to pass in the current location. I think one of the arguments in the builder function is a SourceLocation it should be used as the current location.

@Naddiseo
Copy link
Author

Naddiseo commented Sep 9, 2014

Actually, that fork() idea is really good. My use case is for transpiling, so I want to use an ast tree/builder for the source language, then a separate for the target language (javascript in my case).

I may have a look tomorrow to see if there's an easy way to accomplish this. Do you have any ideas where to start?

benjamn referenced this issue in caplin/global-compiler Oct 8, 2014
The latest recast is using a different parser version which seems to have lost support for the
`File` AST node.
@benjamn benjamn added this to the 0.8.0 milestone Apr 10, 2015
@benjamn benjamn self-assigned this Apr 10, 2015
@benjamn
Copy link
Owner

benjamn commented Apr 10, 2015

I still think this is a great idea, by the way. Hope to work on it for the next minor version (0.8.0).

benjamn added a commit that referenced this issue Aug 7, 2015
This is a temporary workaround for
benjamn/recast#206 until something along the
lines of #57 is implemented.
@jamestalmage
Copy link
Contributor

bump!
0.9.0 maybe?

Personally, I think the core components should be split out into another library (ast-types-base for sake of argument).

I like the syntax, but I think you should pass actual modules (or a custom function) instead of strings. The problem with strings would be accurately resolving them (especially if you do split into a separate module):

var types = require('ast-types-base')(
  require('ast-types/def/core'),
  require('./myCustomDefs'),
  function(types) {
   types.def('MyType')
     //...
  }
);

I do like the fork option discussed, and that seems appropriate for ast-types, but not ast-types-base as I'm proposing. Perhaps strings would make sense there for specifying some of the built in stuff.

This would be the equivalent, less verbose version of above:

require('ast-types').fork(
  'core',
  require('./myCustomDefs'),
  function(types) {
   types.def('MyType')
     //...
  }
);

One problem with the fork method as currently being discussed would be with browserify builds. Browserify will pick up and package everything in def/*, even if you aren't using any of it and creating an entirely custom AST. Splitting into a separate module eliminates that problem for people implementing ground up AST definitions, and the top syntax above could still be used by those targeting browserify to ensure only what they want from def/* will be packaged in the bundle.

@jamestalmage
Copy link
Contributor

@benjamn - anything I can do to move this forward?

@bnjmnt4n
Copy link
Contributor

bnjmnt4n commented Feb 6, 2019

Should probably be closed since #145 was merged nearly 2.5 years (!!!) ago.

@benjamn
Copy link
Owner

benjamn commented Feb 6, 2019

Agreed, though I should mention the move to TypeScript (#300) has complicated this story, since the generated TypeScript declarations are fundamentally less modular/dynamic than the forking system.

@benjamn benjamn closed this as completed Feb 6, 2019
divanutq added a commit to divanutq/ast-types that referenced this issue Aug 1, 2024
This is a temporary workaround for
benjamn/recast#206 until something along the
lines of benjamn/ast-types#57 is implemented.
citiranom2w added a commit to citiranom2w/ast-types that referenced this issue Aug 4, 2024
This is a temporary workaround for
benjamn/recast#206 until something along the
lines of benjamn/ast-types#57 is implemented.
adriaticaz9 added a commit to adriaticaz9/ast-types that referenced this issue Aug 26, 2024
This is a temporary workaround for
benjamn/recast#206 until something along the
lines of benjamn/ast-types#57 is implemented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants