corenova / yang-js

YANG parser and composer
Apache License 2.0
56 stars 18 forks source link

only first include is included #59

Closed sindhukothe closed 6 years ago

sindhukothe commented 7 years ago

When there are multiple include statements in a file, looks like only the first include is compiled. When we encounter the first include, the 'compile' flag is set to false, causing further includes not to be compiled

sekur commented 7 years ago

Can you expand on this scenario?

When we encounter the first missing include, the dynamic import resolver will attempt to find and internally load the dependent submodule with compile flag set to false. Each encounter of a missing include should try to do the same. Once all include dependencies have been internally loaded, then the compile should kick in for the main module which will perform necessary resolve for all included statements as a whole.

sekur commented 7 years ago

Hmm, is this the case when there are nested submodule includes where a submodule also includes additional submodule(s)?

sindhukothe commented 7 years ago

I think you are right. I will have to dig the exact test case again, but meanwhile, Here is the code change that worked. (I changed the compiled code as I am not very familiar with coffee script) in yang.js Replace This: if (e.context.kind === 'include') { opts.compile = false; } dependency = this["import"](this.resolve(basedir, e.context.tag), opts);

With.... (Not a very clean code, but just for experimentation only :) ) var newOpts = opts if (e.context.kind === 'include') { newOpts = {} for (prop in opts) { newOpts[prop] = opts[prop] } newOpts.compile = false } dependency = this["import"](this.resolve(basedir, e.context.tag), newOpts);

sekur commented 7 years ago

Got it, that makes sense. Handling include overrides the shared opts object which then negates the compile value to false for the original module import. I'll make the necessary fix in coffeescript source.

sekur commented 7 years ago

I used Object.assign for this scenario, let me know if this works

sekur commented 6 years ago

Closing