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

Update compiled-grammar.js #12

Closed
wants to merge 7 commits into from
Closed

Update compiled-grammar.js #12

wants to merge 7 commits into from

Conversation

mstijak
Copy link
Contributor

@mstijak mstijak commented Mar 20, 2016

This label was causing a browser error in a webpack generated bundle, so I removed it as it's not referenced anywhere else in the file.
I know that the file is generated, but I don't know how to fix it properly.

Btw, great library. I really like that it doesn't try to be your router. 👍

Fixes browser error.
@rcs
Copy link
Owner

rcs commented Mar 22, 2016

Thanks! could you give me a bit more detail on where you were seeing errors? I'd like at least understand what problem this is causing, even if I can't fix the toolchain for it.

Maybe we can figure that out and come up with a different way to work around it. To get this fixed in this way (changing the compiled file), we'll need to update the compile-parser execution path somehow, so this doesn't get lost and reintroduced when the grammar gets updated.

@rcs
Copy link
Owner

rcs commented Mar 22, 2016

Looking into this more, zaach/jison#305 looks rather promising. When I get a chance, i'll try updating there, or will accept a PR for it.

@mstijak
Copy link
Contributor Author

mstijak commented Mar 23, 2016

You're right. Updating jison to the latest version did the trick.
If you decide to merge this, please also publish a new version on npm.

@rcs
Copy link
Owner

rcs commented Mar 24, 2016

Would you mind squashing these two commits down to one? I'll merge and cut a new version.

Thank you!

@mstijak
Copy link
Contributor Author

mstijak commented Mar 24, 2016

I made a new pull request #14 .

@mstijak mstijak closed this Mar 24, 2016
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 this pull request may close these issues.

3 participants