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

Unable to use the library with Ionic #11

Closed
savanvadalia opened this issue Jun 7, 2017 · 31 comments
Closed

Unable to use the library with Ionic #11

savanvadalia opened this issue Jun 7, 2017 · 31 comments
Assignees

Comments

@savanvadalia
Copy link

Hi There,

Thank you for making this library, can't wait to start using it.
Just have a bit of issue using the lib.

See below error for the screenshot.
I have used the instruction provided to install the lib
npm install json2typescript

image

@andreas-aeschlimann
Copy link
Member

andreas-aeschlimann commented Jun 8, 2017

For starters, there is no index.js in json2typescript - but I don't know why Ionic expects it.

I am not familiar with Ionic because I have never used it. If you want a quick fix, you can try to simple include the necessary *.ts file from our library and call it directly.

I will start with Ionic in a few weeks as well, so I might fix the issue when I encounter it as well. Feel free to let me know if you know how to fix it!

@savanvadalia
Copy link
Author

Hi Andreas,

Thank you for your response.

Regarding Workaround
As per your suggested work around fix, do I just need to include json-convert.ts file in my project?

Regarding the Issue
I think ionic build process ultimately converts *.ts to .js file.

Ionic is a cross-platform hybrid mobile application framework, underneath it still uses the latest Angular version. If you can get it working for ionic it would be great as I would keep getting you enhancements quite easily. I will definitely post a solution if I find one.

Whenever you get a chance to look at the issue here are the steps to reproduce that problem:

  1. Install a kick-starter ionic project steps in below url
    http://ionicframework.com/docs/intro/installation/
  2. Install the json2typescript npm package.
    npm install json2typescript
  3. Open any .ts file in the kick-starter project and import the json2typescript module in any file of the project
    import { JsonConvert } from "json2typescript";
  4. Add below line of code to any method in the above ts file
    JsonConvert.debugMode = true;
    This basically tells the ionic build process that it needs to package the 'json2typescript' lib for the final project bundle.
  5. Save all the changes and do ionic serve from the command line terminal it should show the same error i am getting.

@andreas-aeschlimann
Copy link
Member

Well, in the web (Angular) in the end everything gets translated to JavaScript before runtime as well. You just have to make sure, the JsonConvert TypeScript file gets transpiled into JavaScript as all your other files as well. I don't know how this works in Ionic, but in Angular a simple import is enough.

@gempain
Copy link

gempain commented Jun 13, 2017

In Angular they configured Webpack to package node modules with the bundle, so the problem of not having an index.js is not raised. In Ionic you may fix your problem by importing the lib using a relative path like ../../json2typescript. This will force Ionic to compile and package the lib with your bundle. I don't know exactly how Angular packages node modules imported with no relative math (i.e. import ... from "json2typescript"). I'll need to have a look at their webpack.config.js.

Hope this helps !

@savanvadalia
Copy link
Author

@gempain @andreas-aeschlimann

Thank you for your reply.

Ideally Ionic build scripts automatically picks up 3rd party dependencies from the node_modules if you have a dependency in the code somewhere in your application pretty much the same way as angular.

I tried your suggestion by providing a relative path and it worked perfectly fine
This how i used the relative path in my code
import { JsonConvert, ValueCheckingMode } from '../../../node_modules/json2typescript';

However, I am still in a dilemma whether to implement your solution in my use-case.
So far as a temporary workaround I just copied the json-convert.ts file into my app source code which helps me auto-import the members in any file as I use "Auto-Import" vs-code extension which frees me up about worrying of the relative paths. But the disadvantage of my current workaround I have to always manually copy the newer version of the lib.

On other hand with your suggested approach, Auto-import extension won't work as it will try import to the lib as a proper npm package (import { JsonConvert} from 'json2typescript';) hence I will need to every-time manually type in the relative path based on my current directory. But the good thing is I will get proper updates from the lib with just npm install.

I still hope I can find a proper solution which works smoothly with my current development workflow.

@gempain
Copy link

gempain commented Jun 13, 2017

@savanvadalia
You are welcome :) But I never said this was a good workaround. I just tried to explain why the problem was occurring. Moreover, it is a bad practice to import npm modules with a relative path.

The cleanest you can do IMHO is to copy the json-convert.ts file in your project sources, just as you did. That's why I do for my own projects until the npm version is fixed (I am working on it, probably will create a merge request in the next few days).

As for the problem you mention about having to type the path, I think your IDE should be able to auto import the file once it detects it in your sources (after you copied it). I don't know which IDE you use, but Webstorm does this pretty well.

Don't worry, I'll be trying to fix this issue ASAP as I need this lib to work properly.

@savanvadalia
Copy link
Author

@gempain

May be I confused you, pardon my English :)

I use VS-Code as an IDE and auto-import does work fine if I use json-convert.ts as local copy into my project sources. Auto import does not work for node_modules relative path e.g. import { JsonConvert } from '../../../node_modules/json2typescript'; which as you said is a bad practice and for the same reason IDE might be restricting it.

I hope your fix solves my issue too 👍

@andreas-aeschlimann andreas-aeschlimann self-assigned this Jun 14, 2017
@andreas-aeschlimann
Copy link
Member

I'll add a compiled json-convert.js to the next release. Will this suffice or is there more to do to make it work with Ionic?

@gempain
Copy link

gempain commented Jun 14, 2017

Yes it will :) If you don't mind, I'm working on a pull request on your repo, adding a compiled version and some integration / unit tests with karma. Should be done by the end of the week day :)

@savanvadalia
Copy link
Author

@gempain I see you finished the pull request, you are a legend 🥇
Also I liked that you converted the ValueCheckingMode from a static class to enum as it was working for some versions of typescript and would error out on others.

Once the pull request is merged by @andreas-aeschlimann I will let you know if it solves my problem for the ionic project.

@gempain
Copy link

gempain commented Jun 15, 2017

@savanvadalia You are very welcome :) This is my first pull request, I hope it gets through !

Yes, with Typescript 2.3.4, you cannot use inner classes as it was originally done here. I wanted to change the code as few as possible while focusing on the directory structure and flows.

Please let me know if this fixes your issues. Also, while waiting for the pull request, you can already test to see if it will work for you:

  • git clone https://github.com/gempain/json2typescript/ ~/json2typescript-tmp
  • cd ~/json2typescript-tmp
  • git checkout feature/npm-realignment
  • git pull origin feature/npm-realignment
  • npm install
  • npm run build (creates dist and coverage folders (coverage won't be packed, but if you're curious enough to open it, you'll see it's pretty awesome))
  • npm pack
  • cd /path/to/your/ionic/project
  • npm install ~/json2typescript-tmp/json2typescript-0.9.6.tgz

That's it. Now replace your relative imports with path-less ones and see if it works. You may need to restart your IDE or force it to re-index node packages (at least that's what I had to do).

You should now have type definitions and the compiled version correctly set.

Remember to roll-back to your relative paths until the next release of the lib comes out ;)

@savanvadalia
Copy link
Author

Thanks @gempain I will need some more help from you despite your clear instructions.

This is a bit embarrassing for me as I wasn't able to finish those steps, I have just started using git recently and coming from TFS(source control system like git) environment doesn't help me.

I tried the first step in a directory and it gives below error

So I created a new git repo in the folder by git init and then typed the same command which gives me this error
fatal: Invalid refspec 'C:/Users/Laptop-Savan/json2typescript-tmp'

Can you please help what I am doing wrong here?

@gempain
Copy link

gempain commented Jun 15, 2017

@savanvadalia My mistake, it's git clone ! Now I'm the one embarrassed :(

@savanvadalia
Copy link
Author

Thanks @gempain

After changing pull to clone I was able to generate a json2typescript-0.9.6.tgz successfully.

But doing an npm install under my ionic from the generated .tgz file is now not recognized as a npm module. Just to highlight here previously I was able to access as a npm module.

Also I don't see any dist and typescript definitions files .d.ts folder in the node_modules directory (See below screenshot),
Should I be expecting a dist or typescript definitions folder as a compiled package?

image

Not to mention I did close my IDE and reopened it. Also I created a complete new test ionic project but still wasn't able access the npm module.

@gempain
Copy link

gempain commented Jun 15, 2017

@savanvadalia Run npm run build, then repackage as I explained above. I apologize for forgetting to mention that you have to build the sources before packaging, I will add an npm script in the package.json to create the .tgz package so that this does not happen again. I'll update my comment above.

@andreas-aeschlimann
Copy link
Member

I have to admit I don't like that ValueCheckingType is an exported type, but if this is really the only way we can make this work, ok then.

I will work through the pull request soon and will merge it with what I have planned for the serialization and custom converter feature.

@gempain
Copy link

gempain commented Jun 15, 2017

Don't worry, the ValueCheckingMode enum is not exported ! It's only visible within the file it is declared in :)

@andreas-aeschlimann
Copy link
Member

True, but it needs to be explicitly imported when it is used, right?

@gempain
Copy link

gempain commented Jun 15, 2017

You are right :/ May be we should place the JsonConvert inside a Json2TypeScript module ? This way to use that value you could import Json2Typescript.ValueCheckingMode, but you'd also have Json2Typescript.JsonConvert and so forth for other functions. I think the easiest (and most likely best way) to do it is to export the enum.

@savanvadalia
Copy link
Author

savanvadalia commented Jun 16, 2017

@gempain
npm run build did the trick. FYI, I also had to do npm install before I did npm run build.
And after that guess what I was able to use it in my ionic project Woo Hoo!! 👍

Thanks Geoffroy I learned a lot along the way through this small but effective exercise for me.

@andreas-aeschlimann
Exporting the enum may not be too bad. Yes I understand it will cause a breaking change for people already using this library, but it is should be easy to fix if we mention about the breaking change in the change log.

I will close this issue once the pull request is merged to the master?
If you don't mind please update this thread once the merge is finished.

@gempain
Copy link

gempain commented Jun 16, 2017

@savanvadalia yup :) Always npm install when working with a package.json. I guess I should've also mentioned it ;) Glad your issue is fixed !

@savanvadalia
Copy link
Author

@andreas-aeschlimann Any updates on when @gempain 's pull request be merged?

@andreas-aeschlimann
Copy link
Member

Sorry for being so inactive, I have been very busy and am on a 2-week holiday trip now. I will make further development right after I am back (in 1 week) because I will need the updates as well!

Thanks also for @gempain to help and all your interest.

@gempain
Copy link

gempain commented Jul 4, 2017

You are very welcome :) Have fun on your vacation !

@andreas-aeschlimann
Copy link
Member

I'm going to close this issue, as it should be fixed in the new version. Would appreciate if you tried the current GitHub commit @gempain @savanvadalia.

@andreas-aeschlimann
Copy link
Member

Current version works for me in Ionic and Angular projects.

@gempain
Copy link

gempain commented Jul 31, 2017

Same here :) Sorry for the delay !

@savanvadalia
Copy link
Author

@andreas-aeschlimann Sorry for the extended delay, great work on the new updates. Yes, I can confirm the newer version of json2typescript works fine with ionic. However, now I am running into a strange issue where the code using json2typescript breaks when running in --prod mode. Dev mode works completely fine.

This could be most likely an issue with ionic build process. Although I just printing out the error below just in case if you have any inputs.

vendor.js:1 ERROR Error: Uncaught (in promise): Error: Fatal error in JsonConvert. Failed to map the JSON object to the class "Country" because of a type error.

	Class property: 
		cities

	Expected type: 
		[City]

	JSON property: 
		cities

	JSON type: 
		[object,object]

	JSON value: 
		[{"id":1,"name":"Basel","founded":-200,"beautiful":true,"data":123,"keywords":["Rhine","River"]},{"id":1,"name":"Zurich","founded":0,"beautiful":false,"data":"no","keywords":["Limmat","Lake"]}]

	Reason: Expected type is unknown. You are either missing the decorator @JsonObject (for object mapping) or @JsonConverter (for custom mapping) before your class definition.

Error: Fatal error in JsonConvert. Failed to map the JSON object to the class "Country" because of a type error.

	Class property: 
		cities

	Expected type: 
		[City]

	JSON property: 
		cities

	JSON type: 
		[object,object]

	JSON value: 
		[{"id":1,"name":"Basel","founded":-200,"beautiful":true,"data":123,"keywords":["Rhine","River"]},{"id":1,"name":"Zurich","founded":0,"beautiful":false,"data":"no","keywords":["Limmat","Lake"]}]

	Reason: Expected type is unknown. You are either missing the decorator @JsonObject (for object mapping) or @JsonConverter (for custom mapping) before your class definition.

    at JsonConvert.deserializeObject_loopProperty (http://localhost/cutepuppypics2/www/build/vendor.js:1:724112)
    at JsonConvert.deserializeObject (http://localhost/cutepuppypics2/www/build/vendor.js:1:720425)
    at JsonConvert.deserialize (http://localhost/cutepuppypics2/www/build/vendor.js:1:719618)
    at HomePage.doDeserialization (http://localhost/cutepuppypics2/www/build/main.js:1:13257)
    at new HomePage (http://localhost/cutepuppypics2/www/build/main.js:1:12746)
    at createClass (http://localhost/cutepuppypics2/www/build/vendor.js:1:45080)
    at createDirectiveInstance (http://localhost/cutepuppypics2/www/build/vendor.js:1:41952)
    at createViewNodes (http://localhost/cutepuppypics2/www/build/vendor.js:1:66172)
    at createRootView (http://localhost/cutepuppypics2/www/build/vendor.js:1:64309)
    at Object.createProdRootView [as createRootView] (http://localhost/cutepuppypics2/www/build/vendor.js:1:75485)

    at JsonConvert.deserializeObject_loopProperty (http://localhost/cutepuppypics2/www/build/vendor.js:1:724112)
    at JsonConvert.deserializeObject (http://localhost/cutepuppypics2/www/build/vendor.js:1:720425)
    at JsonConvert.deserialize (http://localhost/cutepuppypics2/www/build/vendor.js:1:719618)
    at HomePage.doDeserialization (http://localhost/cutepuppypics2/www/build/main.js:1:13257)
    at new HomePage (http://localhost/cutepuppypics2/www/build/main.js:1:12746)
    at createClass (http://localhost/cutepuppypics2/www/build/vendor.js:1:45080)
    at createDirectiveInstance (http://localhost/cutepuppypics2/www/build/vendor.js:1:41952)
    at createViewNodes (http://localhost/cutepuppypics2/www/build/vendor.js:1:66172)
    at createRootView (http://localhost/cutepuppypics2/www/build/vendor.js:1:64309)
    at Object.createProdRootView [as createRootView] (http://localhost/cutepuppypics2/www/build/vendor.js:1:75485)
    at c (http://localhost/cutepuppypics2/www/build/polyfills.js:3:13535)
    at Object.reject (http://localhost/cutepuppypics2/www/build/polyfills.js:3:12891)
    at NavControllerBase._fireError (http://localhost/cutepuppypics2/www/build/vendor.js:1:502539)
    at NavControllerBase._failed (http://localhost/cutepuppypics2/www/build/vendor.js:1:502343)
    at http://localhost/cutepuppypics2/www/build/vendor.js:1:503534
    at t.invoke (http://localhost/cutepuppypics2/www/build/polyfills.js:3:9283)
    at Object.onInvoke (http://localhost/cutepuppypics2/www/build/vendor.js:1:122654)
    at t.invoke (http://localhost/cutepuppypics2/www/build/polyfills.js:3:9223)
    at r.run (http://localhost/cutepuppypics2/www/build/polyfills.js:3:4452)
    at http://localhost/cutepuppypics2/www/build/polyfills.js:3:14076

@andreas-aeschlimann
Copy link
Member

I don't know, maybe you can investigate what the --prod flag is doing.

For sure I will look into it as well in the next weeks, because I am also working on an Ionic app now. So far I haven't used the --prod flag yet in combination with json2typescript.

@andreas-aeschlimann
Copy link
Member

andreas-aeschlimann commented Aug 27, 2017

Here you get your exception from:

// Check if attempt and expected was 1-d
if (expectedJsonType instanceof Array === false && value instanceof Array === false) {

    // Check the type
    if (expectedJsonType.hasOwnProperty(Settings.MAPPING_PROPERTY)) { // only decorated custom objects have this injected property
    
    } else if (expectedJsonType === null || expectedJsonType === Object || expectedJsonType === undefined) { // general object
   
    } else if (expectedJsonType === String || expectedJsonType === Number || expectedJsonType === Boolean) { // otherwise check for a primitive type
   
    } else { // other weird types
        throw new Error("\tReason: Expected type is unknown. You are either missing the decorator @JsonObject (for object mapping) or @JsonConverter (for custom mapping) before your class definition.");
    }

}

The weird thing is, the expected type should be array. But instanceof Arrayseems to be false for both expectedJsonType and value, otherwise this error could not be thrown. I have no idea why this would happen, but some debugging there might help.

@savanvadalia
Copy link
Author

--prod mode in ionic does the Ahead-Of-Time compiling for angular, optimisation of JS and CSS file, tree-shaking etc...

I am not sure whether I can debug the code in --prod mode after all the optimisation process the original code is unrecognisable and don't think ionic supports source-maps for --prod mode.

After playing a bit with ionic build process, I was able to find that the issue is with the JS optimisation part. I have opened up an issue for the same with ionic-team here

Please let me know if you come across the same issue with your ionic app in --prod mode

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