-
Notifications
You must be signed in to change notification settings - Fork 46
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
Typing resolutions #102
Comments
A little bit of background on TypeScript typings - originally .d.ts files were included using
When you do Recently there has been change so that you can deliver external declaration files, these are a little different to the one above because the file itself is the module, so the one above would look like:
and when you do a) Look in package.json for "typings" field. If found use that. Rule (b) also applies when doing So this is the difficulty when loading tin the browser, because the plugin doesn't know which packages have typings files. SystemJS resolution can't be used because that needs to resolve to the .js file. Angular2 delivers multiple typings files in the npm package and this makes it impossible to type-check in the browser unless you switch back to using one of the old-style ambient declaration files. Solving the angular2 problem ought to be the priority here I think. If jspm could record any .d.ts files it finds when installing packages so that they were available somewhere in the runtime config then that might work, could jspm-npm do that? Otherwise the typings project may be able to provide this information but its not quite clear how that is going to pan out right now. I hope this is clearer and we can find a way forward. edit: updated the terminology to be in line with the rest of the thread. |
bit more info for background - there are two types of typings files now - "ambient" and "external" - angular2 uses the externals (they are basically es6 modules, more or less) and effectively its a reasonable guarantee that there's 1 its unlikely we'll switch back to the old style (ambient) any time soon / ever, and ideally we'd like to not have to tell consumers of NG2 to do the same. the typings project is for ambient modules only (afaik) and thus we won't be delivering the typings to it's registry for angular2. @guybedford @frankwallis happy to have a pow-wow if you guys want to work through this. |
see also microsoft/TypeScript#6012 - and as of TS 1.8 there's a new (configurable) resolution strategy which is ~mostly JSPM style. |
@robwormald yes maybe I am using wrong terminology so the first example above is "ambient" and the second one "external". In the angular2 case, then, a simple boolean flag to say the package does have external typings would be enough. The plugin would always resolve and fetch external typings for files inside that package. That flag could go in:
So to account for most common scenarios, jspm could retrieve the value of |
@robwormald I don't think I am able to use the new algorithm in the browser, because it needs to search for files. It solves the issue of missing intellisense and being able to use tsc with jspm and external typings though, which is progress. |
@frankwallis let me see what we can do about your first question, we should be able to make one of those options work. The second though - assuming you know that there are typings, no search should be needed, as they are 1 to 1 for the actual source files. (that is, foo.ts becomes foo.js and foo.d.ts on npm) - or am i misunderstanding you? |
@robwormald thanks, I'm really keen to get this working. I need to have a look at the how this new |
@robwormald my understanding is that typings is for external modules. See here and here. So then Angular2 could be added to the registry. Or have I missed something? |
@frankwallis @robwormald thanks for the clear descriptions here. It does sound like Angular would be the nice case to crack here to set the precedent. @robwormald did you say we really can assume the convention then that the typing file for a fully-resolved |
@ximenean as I understand it the aim of the typings project is to allow ambient modules to co-exist with external modules until everything has been moved to external modules. It's all rather confusing though... Some new docs have just arrived on the subject: https://github.com/typings/typings/blob/master/docs/how-typings-makes-external-modules-first-class.md |
@guybedford that is only true for the angular2 package. As an example of another sort of package
This file (build/lib/core.d.ts) will contain bundled typings for the whole package. |
@frankwallis as far as I can tell |
@guybedford yes and in fact the core.d.ts I think it is quite likely that packages which are developed in TypeScript will follow this convention, but packages which are developed in JavaScript may choose to have a single bundled typings file. |
I have managed to get the angular2 example type-checking by telling the plugin which packages have typings files, the configuration looks like this currently:
This works because all imports from these packages specify the module name to be imported (ie |
In jspm 0.17, the main entry point problem here goes away because the main is resolved as part of the normalize step instead of fetched through a pointer module. In cases where you don't have a true value for the typingsMap, perhaps you can just infer from the fact that the plugin fetch hook is being called for a source and load the typing file in parallel at that point (even if it is a 404)? |
@guybedford that's good news about the main entry point issue in 0.17, and I was hoping you would say that! Unfortunately I don't think the plugin fetch hook will always be called in this situation, as the actual file will resolve to a .js file, but TBH I don't think it's a big deal if it can't be fully supported until 0.17, as long as angular2 works. What are your thoughts about putting this configuration into the package config? I am happy to do a PR if you point me in the right direction. I would propose that:
Does this sound ok? |
@frankwallis I'd strongly suggest moving on the 0.17 upgrade path and encouraging users here to as well. I would have published it as stable already, but am going slowly on the stable branch rather to give users time, but individual projects should push forward. The only way to handle a custom field through the package configuration is through meta exactly as in #96 - {
"meta": {
"*.ts": {
"loader": "typescript",
"typescriptOptions": {
"typings": {...}
}
}
}
} Would you still consider that if a TypeScript file is run through the fetch hook, the loader would request the typing by default? If the fetch hook is not run for that specific file (eg for 0.16 mains), then a field like this makes sense certainly. It could potentially also act as an opt-out for the automatic typing loading if that is implemented. My biggest concern is what normalization space this field is acting in? Multiple plain names can refer to the same module, so you'd need to run that name through |
The other thing is even if using normalizeSync, it should probably be a contextual normalization as well to get all the contextual package maps. It's not completely clear though which contextual parent is intended for such an unnormalized typing (with |
If it's a typescript file we don't need any typings, the typings files are only needed for packages delivered as .js files. The plugin needs to know: So the I have put in the |
Ahh, right, thanks for bearing with - sorry it's my distance from these workflows showing in these simple errors. I'm really so grateful you are working all of this out! So the '.js' file has matching '.d.ts' typings usually when it was compiled from TypeScript as separate files then? And thats what Angular and Rxjs do? Is there any indication in the source code of those Would a TypeScript package ever have a typings field for its dependencies? Or is the point here that we associate all typings through overrides to the underlying files? So in jspm 0.17 "meta" can be provided as a field in the package.json (along with all other SystemJS package config options). The code example in my previous comment could be the package.json file itself or the override. Thinking further, could you not just do something like: {
"meta": {
"*.js": {
typings: true
}
}
} So that we use the individual metadata per-module to indicate which files have associated typings? |
might be better with "external" vs "ambient" to denote parallel typings (external)? |
hey no problem, I just want to make sure we get to the right place. So, yes, angular2 and rxjs provide .d.ts files alongside their .js files, but there is no way to figure this out without looking at the files which are in the package. Some other packages can populate a I'm not quite clear on your question, but I think the answer is that a TypeScript package would not have typings for it's dependencies - the plugin should automatically locate those typings in the dependency package if they exist, and feed them into the compiler. This is why it is important for the plugin to know what typings are present. Providing this information in the meta might work, if I can access it at the right time. As an example: somefile.ts:
At this point I normalize "angular/bootstrap" -> "http://jspm_packages/.../bootstrap.js" and I then need to get the meta for that file. I'm not quite sure how to do that? I will have a play around with this and report back. If it works then I think we will have a solution as this meta can be added into the jspm registry presumably, or put in the angular2 package.json file? |
There's a hack that can be used to get the metadata for a package, but it won't work in future versions when the locate hook is deprecated. We can make sure the use case sticks around somehow though. Basically you can do: var metadata = {};
System.locate({ name: 'http://jspm_packages/.../bootstrap.js', metadata: metadata })
.then(function(address) {
// metadata.typings, metadata.typescriptOptions etc should all now be populated correctly,
// respecting the composition of all global metadata, wildcard metadata and package metadata correctly
}); Yes meta can be in the original package.json or the jspm registry. I hate to be introducing "another" way of doing this stuff, but we do just need to let these metadata conventions all fight it out in the wild for a bit I think. |
ok I'll give that a try and report back. I also think that this stuff is likely to change on the typescript side (see microsoft/TypeScript#6950 for example), so I agree it's good not to be over committed at this time. |
@robwormald I don't understand - can you clarify? |
I have added support for specifying
|
Would it be possible to add this meta information to the packages overrides in the jspm registry? So that the consumer of the library doesn't have to set this in his |
@frederikschubert yes that is the plan, I just need to get it working with a manual override first to verify. |
That gives me:
And the
So maybe that is not the right config? |
Yeah, so the right way for this is: "npm:[email protected]": {
"meta": {
"Reflect.js": {
"typings": "Reflect.ts"
}
}
} Maybe this should be reported as |
What is the current status of this issue?
Ambient dependencies seem not to cause problems. What I would like to achieve in my setup is typescript compilation with There is another ticket disabling me on this setup. But I'm not totally sure whether this is a problem with the systemjs-hot-reloader or anything else. |
I think this is working, see the angular2 example What version of jspm are you using? |
I'm using |
It's the same version, it's listed in the root package.json. If you can provide a repro I will take a look. |
I have tried example with Angular2 RC and it won't work. Maybe I'm doing something wrong. Please could you update example to Angular2 RC? |
Hi @frankwallis, sorry for the late response. I recently published ng2-jspm-template. Here the problem ocurrs when you turn on |
@svi3c thanks for sharing your project. The issue with
becomes
It seems that when the packages configuration and map entry for a package happen in different Really this configuration should be setup in the jspm registry but that has not happened yet. In the typescript example project I have moved this configuration into package.json which means that jspm writes it to the config file with the map configuration and makes it easier when updating versions. The error with the hot reloader trying to reload files is happening because the files were originally coming from the bundle and then trying to be reloaded with the |
@sokolovstas the angular2 example is now using rc1 |
Thanks for the clarification and support, @frankwallis! Yes, without using bundles, the hmr works in combination with |
@frankwallis I'm having a similar (?) problem in CockroachDB. Two of our dependencies (redux and reselect) now ship with external typings, so we want plugin-typescript to load them rather than using Any ideas? EDIT: one thing I noticed while debugging this is that EDIT: both packages specify |
I haven't tried it yet (it's late here) but on quick inspection the override should be:
Note the version does not have the Hopefully this will fix it, if not I will take another look. |
@frankwallis updated, but that didn't fix it =/ metadata is still an empty object, too. |
Ok, that configuration only works in [email protected], but in 0.16 you can achieve the same thing by adding global meta configuration in
Alternatively you can also install them using the |
Indeed, both of the solutions you mention work, thanks. However, why is it that this doesn't "Just Work" given that the information is already in reselect's |
The plugin does not have access to the package.json, and even if it did there are some packages which contain typings and do not have a I agree that the configuration is painful, and I am open to better solutions but I just don't see any at the moment. Ideally this configuration will get added into the central jspm registry and then things will "Just Work" for those packages. |
The above typings metadata can also be added with a meta override into the jspm registry for 0.17. Let me know if you'd like help with that at all. |
@guybedford if this information was in the jspm registry, it'd end up in my generated config.js, correct? If that's the case, why doesn't (or can't) jspm read package.json and populate the same thing? Seems to me that the jspm registry ought to be for overriding packages' own information as opposed to housing copies thereof. |
@frankwallis, you mentioned that "the algorithm TypeScript uses for finding external typings cannot be used from within a browser as it involves trawling the file system". Would it be possible to support this for precompilation builds and only require the additional configuration for the browser? |
@bmayen it would be possible, but I am reluctant to go down that route as it would mean the file resolution worked completely differently in the 2 scenarios and that would be a constant source of issues. It seems to me that if you are using that workflow then you may be better off type-checking with |
I have added a new compiler option Also plugin-typescript now supports the I am going to close this issue now, if you find issues with either of these features then please raise them separately. |
It sounds like the resolution problems tackled in typings/typings#123 are critical to this plugin's type checking feature. I'd be really interested to see what can be done to make this work, if you don't mind helping me understand this area a little better.
Firstly, jspm-npm will handle conversion of npm packages converting the Node resolution algorithm into the SystemJS resolution algorithm.
Are you saying that typings themselves are written assuming the Node resolution algorithm? Which assumptions are these usually based on? For example, are extensions usually used? Do directory requires or json requires get used? How exactly does SystemJS's naive import not work with this resolution currently?
The text was updated successfully, but these errors were encountered: