Closed letmaik closed 7 years ago
Fixing this seems easy.
I should only change the UMD pattern here
to require("chevrotain")
instead of require('../../lib/chevrotain')
What I do not understand though is why you are doing
var diag = require('chevrotain/diagrams/src/main');
The code that generates the diagrams uses the railroad diagrams 3rd party lib.
This code has dependencies to the DOM and I doubt it will run on node.js. It is more accurate to say that that diagrams are not being generated (as new files created) rather that they are being rendered.
Due to these limitations the current diagrams workflow is normally.
Is your workflow different?
I don't run the diagramming code in node.js, but use browserify for that particular diagramming file. But it is the same code base that otherwise runs on node.js. So what I do is I run browserify like that:
$ browserify diagram.js -u wordnet-db -u lapack -o diagram.bundle.js
(have to exclude wordnet-db and lapack as those optional chevrotain dependencies get picked up by browserify and would fail since I don't use them and haven't installed them) see comment below
And then I have the following HTML:
<!DOCTYPE html>
<meta charset="utf-8">
<style>
body {
background-color: hsl(30, 20%, 95%)
}
</style>
<link rel='stylesheet' href='../node_modules/chevrotain/diagrams/diagrams.css'>
<body>
<div id="diagrams" align="center"></div>
<script src='diagram.bundle.js'></script>
</body>
The diagram.js file looks like that:
var main = require('./diagrams/main'); // my local fixed copy
var MyParser = require('./parser');
var parserInstanceToDraw = new MyParser([]);
var diagramsDiv = document.getElementById("diagrams");
main.drawDiagramsFromParserInstance(parserInstanceToDraw, diagramsDiv);
o.k.
So you are building your template in a different way, no worries I see no reason not to support this as well. I will try to modify the diagrams's code UMD pattern to use require("chevrotain")
so it would resolve to the same node module when browserifying and thus will avoid
having two separate prototype chains when running in node.js which causes the instanceof operator to fail.
(have to exclude wordnet-db and lapack as those optional chevrotain dependencies get picked up by browserify and would fail since I don't use them and haven't installed them)
Chevrotain has optional dependencies ? This is new to me... 😄 did you mean something else?
Thanks! Sounds good.
(have to exclude wordnet-db and lapack as those optional chevrotain dependencies get picked up by browserify and would fail since I don't use them and haven't installed them)
Chevrotain has optional dependencies ? This is new to me... :smile: did you mean something else?
Sorry, mixed something up there. These are actually optional dependencies of the natural
library. My fault.
Allright @letmaik.
Can you please check if commit https://github.com/SAP/chevrotain/commit/5c10ff88ac443793576ad71bb0056a9f783e374a fixes your issue? (just perform the same modification in node_modules/chevrotain/diagrams)
Seems to work locally for me on node.js version 6, but I am just requiring it, I do not perform packaging. Do you have another version of node.js? I will test this tomorrow on node.js version 4 as well before releasing.
I am wondering if your method of packaging and creating the diagrams is easier to use versus the one documented in Chevrotain. or if possibly there is no "best" way and each user should be left to his/hers own devices?
Yes that works for me.
I think my method is easier the moment you have a few more dependencies in your parser module, like using additional libraries or depending on other modules in your project. So if you use node and npm anyway, then I think this is the proper way. If you don't, it may be too much overhead to set it up.
@letmaik version 0.13.2 is out on npm which includes the bug fix.
Regarding easier packaging for the diagrams, I think this would be resolved once I find the time to implement serialization and de-serialization of the grammar, thus allowing rendering the diagrams without browser packaging.
Instead simply using a script to generate the serialized form and load it in the browser for rendering purposes. So no need to package your entire parser including its dependencies for use in the browser.
Thanks for the quick release!
I agree about your plan with generating diagrams directly. This would make things a lot simpler, especially when autogenerating bits of documentation.
Thanks for the quick release!
Releasing is easy when you are lazy and automate everything 😄
And thanks for the feedback.
I'm using chevrotain on nodejs and wanted to generate a diagram for a parser using nodejs workflows. I stumbled upon a serious problem though, which is in the use of
instanceof
in chevrotain code and how the chevrotain library is require'd within the diagram code.When I do
var chevrotain = require('chevrotain')
then this effectively includeslib/src/api.js
from chevrotain because this is what the"main"
field of chevrotain's package.json references. All fine. However, when I then dovar diag = require('chevrotain/diagrams/src/main')
then this internally does arequire('../../lib/chevrotain')
which effectively loads a different chevrotain library.Working with two different chevrotain objects breaks badly in cases when chevrotain does an
instanceof
check like here or here.As a temporary fix, I copied the /diagrams/src folder into my project and changed the
diagrams_builder.js
file fromto
So the important bit is the second require.
chevrotain
instead of../../lib/chevrotain
. With that it works since it uses a single library only. I am not sure if this solution is the best one and works for everyone, but it works for me.