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

Next Major: Simplifying the API, docs and the plugins, smaller codebase, TypeScript support #65

Open
tunnckoCore opened this issue May 15, 2018 · 6 comments · May be fixed by #67 or #72
Open

Next Major: Simplifying the API, docs and the plugins, smaller codebase, TypeScript support #65

tunnckoCore opened this issue May 15, 2018 · 6 comments · May be fixed by #67 or #72
Labels
Pkg: parse-function Priority: High After critical issues are fixed, these should be dealt with before any further issues. Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. Type: Enhancement Most issues will probably be for additions or changes. Expected that this will result in a PR.

Comments

@tunnckoCore
Copy link
Owner

What about that?

import { parse } from 'parse-function'

const pluginsArray = [
  (node, result) => {},
  (astNode, result) => {},
]

const result = parse(string|Function, parserOptions, pluginsArray)

// or without passing parser options
const result = parse(string|Function, pluginsArray)

We are just removing the extra step of creating "app" instance and then calling parse.
Also removing the use so called "smart plugins", we don't need to change anything to that "app" instance.

So now plugins are more simpler and flat, instead of (app) => (node, result) => {}

Should be noted that we can't just add plugins property to the parserOptions, because will conflict with babylon for example.

@ericmorand
Copy link

ericmorand commented Oct 12, 2018

Would be great.

Also, while we are at it, some other things could make using parse-function much easier: TypeScript definitions.

Currently it's difficult to use parse-function in TypeScript project because of the lack of definition. We currently have to import parse-function using require instead of import ... from ... because of the lack of definition and it makes the compiled code userequire too. Why is this a problem? Because of this:

webpack/webpack#8171

Webpack will resolve the require to dist/index.es.js and the code will fail since dist/index.es.js export a default property that holds the parser constructor while dist/index.js export the parser constructor.

By having a definition, we would be able to use the import ... from ... in our TypeScript code and have the import ... from ... also used in the compiled code.

See that issue to see the impact of that on a real project:

https://github.com/ericmorand/twing/issues/284

@tunnckoCore
Copy link
Owner Author

I'm not so in TypeScript, so definitions are always welcome as PR.
I may add them by myself but i'm not sure.

@ericmorand
Copy link

ericmorand commented Oct 12, 2018

I can take care of that. Since your project is not written in TS, it's better to host then in DefinitelyTyped repository. If you don't mind, I'll add them there.

@tunnckoCore
Copy link
Owner Author

Great, yea! :)

@tunnckoCore
Copy link
Owner Author

tunnckoCore commented Oct 18, 2019

@ericmorand

I'll handle that. I already moved the source code to new home at my monorepo https://github.com/tunnckoCore/opensource/tree/master/packages/parse-function, so it will eventually get written in typescript.

GitHub
Delivering delightful digital solutions. JavaScript Open Source Monorepo, semantically versioned following @conventional-commits. Fully powered by Jest, @babel TypeScript, @airbnb @eslint + @pretti...

@tunnckoCore tunnckoCore transferred this issue from tunnckoCore/parse-function Oct 18, 2019
@tunnckoCore tunnckoCore changed the title Next Major: Simplifying the API, docs and the plugins, smaller codebase Next Major: Simplifying the API, docs and the plugins, smaller codebase, TypeScript support Oct 18, 2019
@tunnckoCore tunnckoCore added Pkg: parse-function Priority: High After critical issues are fixed, these should be dealt with before any further issues. Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. Type: Enhancement Most issues will probably be for additions or changes. Expected that this will result in a PR. labels Oct 18, 2019
@tunnckoCore
Copy link
Owner Author

tunnckoCore commented Oct 24, 2019

Final API for v6, defaults to use @babel/parser if not options.parse given.

import { parse as acornParse } from 'acorn';
import { parseFunction } from '.';

// `node` is an AST Node
function bobbyPlugin(node, result) {
  const bobby = 'bobby';

  return { ...result, bobby };
}

function barryPlugin(node, result) {
  return { ...result, barry: 'barry barry' };
}

const result = parseFunction('function foo(bar, qux) {}', {
  parse: acornParse,
  plugins: [bobbyPlugin, barryPlugin], // supports array of plugins
  parserOptions: {},
});

console.log(result);

It also has some TypeScript support. For more, PRs welcome.

tunnckoCore added a commit that referenced this issue Oct 24, 2019
BREAKING CHANGE: exports named `parseFunction`,

For more see #65 (comment)

Signed-off-by: Charlike Mike Reagent <[email protected]>
@tunnckoCore tunnckoCore linked a pull request Oct 24, 2019 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pkg: parse-function Priority: High After critical issues are fixed, these should be dealt with before any further issues. Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. Type: Enhancement Most issues will probably be for additions or changes. Expected that this will result in a PR.
Projects
None yet
2 participants