-
Notifications
You must be signed in to change notification settings - Fork 3
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
TypeScript implementation #20
base: v1.2.0
Are you sure you want to change the base?
Conversation
I fixed the underlying issues here, which I believe did stem from changes in 2.0.0. This leads me to think if first issue listed above, regarding imports for interfaces like I found that a similar issue, where - <elementImport xmi:id="_SQ0AwOqNEeyCo6a7S9M9vA">
- <importedElement xmi:type="uml:Class" href="skos.uml#_XhC24E2LEe2Pq5gmFYw1MQ"/>
+ <elementImport xmi:id="_6HIqYE76Ee2TpoYuSsmzTA">
+ <importedElement xmi:type="uml:Interface" href="skos.uml#_r06YgE1mEe2Pq5gmFYw1MQ"/> I feel I'm starting to exceed my proper understanding of the UML model and don't want to be introducing changes like this without additional feedback. I'll see what headway I can make with the last issue, regarding tests, but will mark this ready for review so that others can suggest improvements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work @jgaehring.
I wrote some queries to automatically import what's needed. This will ease the generator code and also the UML model like it was proposed here #16.
I would need push access to your fork @jgaehring in order to add my commits to this PR, could you allow me? For docs see here.
I was able to run (failing) tests.
Here are some more tasks that need to be handled:
- Auto-imports (I should be allowed to push my commits to this PR)
- SocialMedia has two getUrl method data-model-uml#12 (I can take it)
- Missing "Semantic" stereotype on ISocialMedia and IPhoneNumber interfaces data-model-uml#13 (I can take it)
- Abstract sub classes should not re-implement methods already implemented in parents (like some methods in
Agent
sub classes). - Old references to static context should be replaced with https://www.datafoodconsortium.org (in the
Connector
class and in tests)
Oh great, all those points make sense. Thanks, @lecoqlibre! 🙏
I believe this is enabled now? If you have difficulty pushing to it let me know.
Ah, yes I saw this in the original TS codegen when I tried to run the tests there too and wondered. I can do a quick search and replace easily enough and push that up first.
Got it, I think I can fix that easily enough. |
I just realized that some places in the PHP static code it references the JSON and RDF data from other DFC repositories, like below: connector-codegen/src/org/datafoodconsortium/connector/codegen/php/static/src/Connector.php Lines 17 to 22 in c177f39
And in the Ruby static code here: Lines 17 to 22 in c177f39
I gather these are fulfilling the same goal as I'll try replacing those as best as I can figure out and push it on top of your two latest commits (or perhaps a separate fork, or both), that way you can see the diffs and it should be more clear. |
Here's the commit with the substitutions I made for |
I'm having a devil of a time with the tests. There are likely other issues (see * below), but a major hindrance seems to be related to the Jest test runner itself and using ECMAScript Modules instead of CommonJS, which seems linked to use of the
Here's what the error message looks for one of the tests. Note it refers to a line
Have you encountered anything like this, @lecoqlibre? * = The other issue I referred to is an error that reads |
Yes absolutely, same here. I encountered this issue in the previous version of the TS connector. I think I found a workaround but don't know if it can work for the new version and the evolution of Jest. Are you familiar with other test framework that we might use instead of Jest?
I think it's related to the prefixes we are now using in the connector. I'm working on the Semantizer to add a |
Actually, I've just about got the native Node.js test runner working. It was introduced with Node v.18 (LTS) back in 2022, so it's stable. I was about ready to say I fixed it, but I wanted to try it out with a clean build of the generated code, and now I'm seeing some weird syntax errors in the generated TypeScript. I haven't changed anything on the Acceleo side, just the static files in |
Ohhh, I think this might be related to when I deleted the
However, it has rebuilt the binaries after that first try, so then when I run the generator after that, it completes the process and generates the connector files, but with tons of syntax errors in the generated code. There are also some new errors in the Acceleo console, which may be related to that initial
So I guess this is something I broke with my config when I deleted those binaries the first time? And for some reason it's not able to rebuild those binaries without error when I try to start over with a clean slate? I need to step away for a little bit, but in the meantime, I'm pushing two commits that swap out the Jest test runner for the native Node test runner. Maybe if you pull those down, you can get them to work for you? |
Oh, and I've got one more commit I'm not pushing onto this branch just yet, b/c I didn't want to muddy the waters any further, but I removed some unnecessary jgaehring/connector-codegen@typescript...jgaehring:connector-codegen:async-tests |
My last commit edaf89f integrates the dev (alpha.3) version of Semantizer. I changed the Semantizer dependency type to a GitHub repo, this way we can run @jgaehring I also updated dependencies of Also, note that we should use the UML model from the next branch of the data-model-uml repo when generating. I merged your commits about using As the Semantizer has now a dedicated class which act like an environment, we should move in the future some components from the connector to the Semantizer (like I did for the PHP version). Especially import/export, store, factories. |
So far we get all the tests passing! Some tasks remain:
@RaggedStaff We should check that the transformation loop suits the needs. Have we some use cases from the users? I did a simple test in the PlannedTranformation test file. I have trouble to see for instance how we can do a combine transformation as the |
I've got a start at the test generation (2c8507a). So far I'm just testing the constructor. There are a total of 109 tests with 99 passing and 10 failing. I've put the relevant tests and the full results up on this gist: https://gist.github.com/jgaehring/bde27fe6991a289cc7016ed77bc1120e There seem to be three kinds of errors that I'm not yet sure how to resolve:
I have to put this down for now, might not be able to pick it back up til later this week. It's taken me a bit longer than anticipated (what else is new), but if these issues can be knocked out, hopefully tests for the getters and setters will follow in short order. 🤞 |
…emented in parents
This should alleviate the issues regarding Jest and ECMAScript Imports. Docs: https://nodejs.org/dist/latest-v20.x/docs/api/test.html#test-runner
@lecoqlibre / @jgaehring is this ready to merge ? |
This should represent all the main features for the implementation, to eventually resolve datafoodconsortium/connector-typescript#4, except for a few outstanding issues:
IPerson
,ISocialMedia
, andIPhoneNumber
, into classesAgent
,Enterprise
, etc. These seem to coincide with the features added in 2.1.0, or maybe that's just a red herring.DefinedProducts
, specifically getters for multiple properties that implement subclasses ofICharacteristic
, such asgetPhysicalCharacteristics
,getAllergenCharacteristics
. I haven't identified any others. Possibly something to do with theISKOSConcept
, which also seems to be having some issues getting imported correctly for the interface definitions."dfc-b:Catalog"
).Regarding those prefix mappings, I see that the PHP semantizer does have a
setPrefix
method that sets it for the connector:connector-codegen/src/org/datafoodconsortium/connector/codegen/php/static/src/Connector.php
Lines 17 to 21 in c177f39
But as far as I can tell, there's no such analog yet in the TypeScript semantizer.
I'm leaving this as a draft for now until those issues are resolved or worked around, but if easier to address them in the meantime by marking it for review, @lecoqlibre or @RaggedStaff, please go ahead!