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

this[type] is not a function #333

Open
ghost opened this issue Feb 4, 2017 · 22 comments · May be fixed by #391
Open

this[type] is not a function #333

ghost opened this issue Feb 4, 2017 · 22 comments · May be fixed by #391

Comments

@ghost
Copy link

ghost commented Feb 4, 2017

I'm need to modify js file. So I parse my JSX with esprima, make some changes and then I'm trying to code AST into js. But I have this error this[type] is not a function

Code

import React from 'react';
import { Route, IndexRoute } from 'react-router';
import { Map } from 'immutable';

import App from './containers/App';

export const pages = new Map({
    index: '/'

});

/*eslint-disable react/jsx-max-props-per-line*/
/*eslint-disable react/jsx-sort-props*/
/*eslint-disable max-len*/
export default (
    <Route path = { pages.get('index') } component = { App }>
        <IndexRoute component = { App } />
    </Route>
    
);

Parsing file:

        const file = fs.readFileSync(routes, 'utf8');
        const parsedFile = esprima.parse(file, {
            sourceType: 'module',
            jsx: true,
            comment: true
        });
        let token = false;
        for (let i = 0; i < parsedFile.body.length; i++) {
            if (parsedFile.body[i].type === 'ExportNamedDeclaration') {
                const declarations = parsedFile.body[i].declaration.declarations;

                for (let j = 0; j < declarations.length; j++) {
                    if (declarations[j].id.name === 'pages') {
                        const properties = declarations[j].init.arguments[0].properties;
                        properties.push({
                            type: 'Property',
                            key: {type: 'Identifier', name: 'test'},
                            computed: false,
                            value: {type: 'Literal', value: '/test'},
                            kind: 'init',
                            method: false,
                            shorthand: false
                        });
                    }
                }
            }
            token = true;
        }

Escoding:

        const code = escodegen.generate(parsedFile, {
            format: {
                indent: {
                    style: '    ',
                    base: 0,
                    adjustMultilineComment: true
                },
                newline: '\n',
                space: ' ',
                json: false,
                renumber: false,
                hexadecimal: false,
                quotes: 'single',
                escapeless: false,
                compact: false,
                parentheses: true,
                semicolons: true,
                safeConcatenation: false
            },
            moz: {
                starlessGenerator: false,
                parenthesizedComprehensionBlock: false,
                comprehensionExpressionStartsWithAssignment: false
            },
            parse: null,
            comment: true,
            sourceMap: undefined,
            sourceMapRoot: null,
            sourceMapWithCode: false,
            file: undefined,
            directive: false,
            verbatim: undefined
        });
@papandreou
Copy link
Contributor

escodegen doesn't currently support serializing the JSX-related AST nodes. There's an old fork, but that only works with esprima-fb, which is no longer maintained.

It seems like it would be possible to move escodegen-jsx forward, adapt to the AST format differences, and turn it into a PR against escodegen.

@Constellation, @michaelficarra, would that be a welcome contribution?

@michaelficarra
Copy link
Member

@papandreou Where are the JSX AST nodes specified? How stable are they? What do people use today to codegen JSX code? Are there any estree-based parsers that also support JSX?

@papandreou
Copy link
Contributor

Where are the JSX AST nodes specified?

It looks like this is as authoritative as it gets: https://github.com/facebook/jsx/blob/master/AST.md

How stable are they?

Historically, quite stable. Latest addition was JSXSpreadChild in June 2016. I have to admit that I'm not well informed about whether more stuff is coming up, though.

What do people use today to codegen JSX code?

There's babel-generator.

From a quick glance eslint --fix sidesteps the need by only doing text-based replacements in the rules that apply to JSX.

Prettier comes with its own code generator that supports the JSX extensions.

Are there any estree-based parsers that also support JSX?

esprima 3.0.0+
acorn-jsx
I don't know if babylon counts, as they're deviating from estree a bit.

@nadavye
Copy link

nadavye commented Dec 9, 2017

Any update on this issue?
Do you plan to resolve it soon?

@KathiresanRamkumar95
Copy link

Any update on this ?

Thanks !

@papandreou
Copy link
Contributor

@michaelficarra, any thoughts on what I wrote in #333 (comment) wrt. whether JSX support would be a welcome addition?

@erquhart
Copy link

erquhart commented Apr 5, 2019

@michaelficarra bump ^^ inquiring minds want to know :)

@michaelficarra
Copy link
Member

@papandreou @erquhart Since JSX seems to be a thing that people are continuing to do, I'm okay with having escodegen support it up to the point that it doesn't sacrifice the original goal of escodegen being able to correctly, unambiguously, and minimally generate source text for any estree format AST. I'm not volunteering to do the work to add that support, but I'll review/merge PRs to the best of my ability.

@erquhart
Copy link

erquhart commented Apr 5, 2019

Glad to hear you're open to it! Reviewing and merging is plenty of work on it's own, thanks for being up to it.

It looks like the general test suite is against an in-repo copy of esprima 1.0.0, but JSX support requires at least 3.0. If we prefer not to test against the esprima npm dependency, we could paste in 3.0 or 4.0 and add a dedicated suite for JSX mirroring the approach in test/test.js. Sound right?

@michaelficarra
Copy link
Member

I'm not sure why we wouldn't just use an npm dev dependency.

@erquhart
Copy link

erquhart commented Apr 5, 2019

I'm not either, but your current test suites don't. Just looking to match whatever you as maintainers prefer. I assumed you were pinned to that test copy of 1.0.0 as a common denominator (and non-moving target). Should we just use npm instead of that copy, and add the JSX tests to the general suite, or leave the general suite as is and use the npm copy for a separate JSX suite?

@erquhart
Copy link

erquhart commented Apr 5, 2019

The only issue I'm seeing that's complicating things a bit is how esprima returns Literal nodes with their value and raw values wrapped in literal single quotes (inside of double quotes):

+ expected - actual

   "value": {
       "type": "Literal",
-      "value": "'string'",
-      "raw": "\"'string'\""
+      "value": "string",
+      "raw": "\"string\""

This is also one of two test failures that surfaces when using the npm copy of esprima in the general test.js suite. (The other failure is esprima no longer returning a leading 0 for less-than-one numeric literals, ".14" instead of "0.14".)

For now I'm going to just return the value from literals within JSX so tests can pass, but we'll need to consider how to address this before merge time. I could be missing something obvious too, maybe an option needs to be set for compatibility with newer esprima?

@erquhart erquhart linked a pull request Apr 5, 2019 that will close this issue
3 tasks
@erquhart
Copy link

erquhart commented Apr 5, 2019

PR is up with full support, outstanding points of discussion noted in the OP.

@papandreou you're pretty great with syntax related things, if you're still interested in this and have time to give this a pass to see if I missed anything, please do! You can open a PR against my branch to address gaps as well.

@AwesomeDevin
Copy link

AwesomeDevin commented Aug 29, 2019

How can i resolve it , please !!!

@papandreou
Copy link
Contributor

@AwesomeDevin, you can take @erquhart's PR for a spin and help him QA and finish it :)

@erquhart
Copy link

erquhart commented Sep 4, 2019

Yeah it's definitely not dead, I just got temporarily pulled away from the work that needed it. The PR does need some improvements, I noticed deficiencies compared to a prior JSX PR that was more robust, but that didn't make it through for some reason - and now I cannot find it for the life of me. Perhaps it was against another ES stringification lib.

@sag1v
Copy link

sag1v commented Jul 3, 2022

@papandreou @erquhart Are you still considering supporting JSX?

@papandreou
Copy link
Contributor

I'm not actively working on it.

@sag1v
Copy link

sag1v commented Jul 7, 2022

I'm not actively working on it.

Oh sorry to hear that 😞 so is it safe to say that this project is not maintained anymore? If so, i think it would be nice if it was mentioned in the readme.
Nevertheless, thanks for this great project 🙏

@papandreou
Copy link
Contributor

I'm not a maintainer of this project, and I was only talking about the JSX support, which @erquhart initiated work on. I think you're jumping to conclusions :)

@sag1v
Copy link

sag1v commented Jul 9, 2022

@papandreou Yeah sorry! I tagged the wrong person 😄 I meant to tag @michaelficarra

@smol-honk
Copy link

Would we be able to sync this bad boy in here? https://github.com/wallabyjs/escodegen

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

Successfully merging a pull request may close this issue.

8 participants