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

Reserved keywords as attributes produce error #62

Open
Fuco1 opened this issue Mar 2, 2018 · 9 comments
Open

Reserved keywords as attributes produce error #62

Fuco1 opened this issue Mar 2, 2018 · 9 comments

Comments

@Fuco1
Copy link

Fuco1 commented Mar 2, 2018

I use babel to translate jsx syntax into my own custom components not React (you can do this by including a pragma such as /** @jsx generate */ at the top of the file, generate is then the method fed the translated data).

I have this code

<amount width="25" minimumFractionDigits="4" class="text-dark font-weight-normal">
   {elk.SupplyPeak.Price}
</amount>

I get a parse error on class because it is reserved js word. Quick peak into rjsx-parse-identifier and js2-must-match-name shows that it tests for keywords and produces an error. Maybe we should be more permissive here?

@felipeochoa
Copy link
Owner

I just gave this a try, but the problem is most reserved keywords are not parsed as NAME tokens, but rather their own tokens (js2-CLASS, js2-EXPORT, etc.). If there were a way to handle these generically, I'd be open to it. All that would be required would be writing a rjsx-must-match-name while still using the js2 tokenizer without introducing a long, brittle list of name-like tokens.

@Fuco1
Copy link
Author

Fuco1 commented Mar 7, 2018

I've tried hacking on that and got it to parse class as attribute but then the quoted value afterwards did not. It's quite a mess :/

@knobo
Copy link

knobo commented Mar 9, 2019

I was thinking maybe it would be easy just adding KEYWORD_IS_NAME

Something like this:

(defun rjsx-match-token (match &optional dont-unget)
  "Get next token and return t if it matches MATCH, a bytecode.
Returns nil and consumes nothing if MATCH is not the next token."
  (setf js2-ti-lookahead 0)
  (if (/= (js2-get-token 'KEYWORD_IS_NAME) match)
      (ignore (unless dont-unget (js2-unget-token)))
    t))

(defun rjsx-must-match-name (msg-id)
  (if (rjsx-match-token js2-NAME nil)
      t
    (if (eq (js2-current-token-type) js2-RESERVED)
        (js2-report-error "msg.reserved.id" (js2-current-token-string))
      (js2-report-error msg-id)
      (js2-unget-token))
    nil))

(cl-defun rjsx-parse-identifier (&optional face &key (allow-ns t))
  "Parse a possibly namespaced identifier and fontify with FACE if given.
Returns a `js2-error-node' if unable to parse.  If the &key
argument ALLOW-NS is nil, does not allow namespaced names."
  (if (rjsx-must-match-name "msg.bad.jsx.ident")
      (let ((pn (make-rjsx-identifier))

But there is too many things going on that I don't understand. And this did not help much.

@knobo
Copy link

knobo commented Mar 11, 2019

If there is anything I could try out, give me some hints.

@felipeochoa
Copy link
Owner

IIRC, the KEYWORD_IS_NAME helps in some cases, but there are a lot of keywords it still parses specially. If you're asking about how rjsx-parse-identifier works (which I agree is a mess), the relevant bits are:

(cl-defun rjsx-parse-identifier (&optional face &key (allow-ns t))
  ;; snip
  (if (rjsx-must-match-name "msg.bad.jsx.ident")
          (if (js2-match-token js2-NAME)
  ;; snip
  (if (js2-match-token js2-NAME)
              (if (eq prev-token-end (js2-current-token-beg))
                  (progn (push (js2-current-token-string) name-parts)
                         (setq prev-token-end (js2-current-token-end)
                               name-start (or name-start (js2-current-token-beg))))
                (js2-unget-token)
                (setq continue nil))
  ;; snip

The complexity elsewhere comes from checking for namespaces and allowing dashes in names. (So it's stitching one token out of several js2-NAME, js2-COLON, js2-SUB, and js2-ASSIGN_SUB)

Does that help?

@knobo
Copy link

knobo commented Mar 12, 2019

How about not using js2 to parse the attribute name, and force js2 somehow to skip forward as much as we parse. Just check for {...var} and make js2 parse that for us?

@knobo
Copy link

knobo commented Mar 13, 2019

I mean, since html/xml is not javascript maybe not use javascript parser to parse it.
I don't want to try parse it without js2 if that would not be accepted back in to rjsx. That's why I ask.

@felipeochoa
Copy link
Owner

The issue is not the parser but the lexer. But either way, yes, that's probably right. We already manipulate the lexer internals to handle -=, so we might as well do it all ourselves. If you're willing to dig into js2 to make this happen, by all means!

@knobo
Copy link

knobo commented Mar 26, 2019

So it's js2 that has to be fixed or improved?

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

No branches or pull requests

3 participants