-
-
Notifications
You must be signed in to change notification settings - Fork 55
A few cleanups and enhancements #32
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
Conversation
IntelliJ IDEA couldn't find modules inside tns-core-modules - this fixes the issue (it was also in Nathan's original seed).
The file is no longer in the repo :)
We need it to link native platform declarations to prevent errors when `tns run <platforms>` runs
Without this the TS compiler will complain when fi 'android.' is encountered.
Those are only required on npm
demo/tsconfig.json
Outdated
| "baseUrl": ".", | ||
| "paths": { | ||
| "*": [ | ||
| "./node_modules/tns-core-modules/*", |
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.
I suggest not use use paths to the core modules but write the full imports (import { .. } from "tns-core-modules/ui/view") because of issues with the Angular AoT compiler.
angular/angular-cli#5618
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.
AFAIK that's fixed by adding skipLibCheck: true to tsconfig.json (what this PR does also). I had that same issue and adding that property made webpack behave nicely.
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.
I think that skipLibCheck resolves another problem which happen to fail with the same error (heap out of memory).
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.
That's true. But is the recommendation for all NativeScript users that want to use AoT to prefix their modules (in both apps and plugins)? I don't see that happening in the Groceries sample for instance so this confuses me.
Is there a way to reproduce the problem with my suggested changes?
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.
The AoT compilation in Groceries (as well as the other NS apps, maintained by the team) is used only with webpack. When bundling with webpack, we use the tsconfig.aot.json configuration for TypeScript. The paths, specified in the tsconfig.aot.json, are different from the ones in tsconfig.json. I'll call them full paths from now on. Using the full paths helps prevent the memory problem when bundling. I wrote more about that in the already mentioned issue - angular/angular-cli#5618 (comment).
You may ask why don't we use the same approach for plugins. I think that adding the full tsconfig paths in all plugins is not a sustainable solution. After all, we may add new folders to tns-core-modules, change the names of the existing ones or even drop some of them. If that happens, the plugin authors will have to go over all their plugins' tsconfigs and update them one by one or they won't be able to compile them with tsc anymore.
But what about the apps that have the full paths? Well, as I mentioned, they are placed in the tsconfig.aot.json file - and this one is added by the nativescript-dev-webpack plugin. Then, everyone that has that file, uses the plugin. If a change is required in the full paths, we can use the ns-webpack to modify the tsconfig.aot.json file (on postinstall, for example) and fix the user's project.
After all, we hope that the memory problem when using @ngtools/webpack gets resolved soon. But before that happens, I think it's best to recommend for plugin authors to use full imports - tns-core-modules/....
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.
I see what you mean. I'm just wondering if you webpack an app that doesn't use full paths (but has a tsconfig.aot.json file): what difference would it make if the plugin uses full paths or not? I'd assume the paths are resolved by webpack similarly as the app's code.
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.
The full imports will get resolved faster during build time, but the difference will be negligible unless there are hundreds of thousands imports from tns-core-modules. It's just because TS will first try to find the import as-is and then go through each of the path mappings.
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.
Alrighty! Thanks for taking the time to explain the reasoning to me, much appreciated. I've reverted the relevant bits.
Hi,
I was checking out the latest seed code today and I was 😃 to find that the linking and watching works REALLY WELL these days!
I stumbled upon a few things that I'd like to see corrected, so here's a PR for these thingies:
tns-core-modules/prefix when importing core modules in the plugin and demo code. I don't think that's what real apps do so we shouldn't make it different for plugin authors IMO..gitignoreso it no longer adds<plugin>.d.tsfiles to GitHub (those are only required for npm, and are generated bytsclocally when changing plugin code). Also removed a few redundant lines.tns run iosin the demo folder the TypeScript compiler complained about native references (android.). These could be ignored but it may confuse people, so I've added the platform declarations to the demo as well.rm -rftorimrafinpackage.jsonfor Windows compatibility.Feel free to squash these commits as I was a bit commit-happy :)