Skip to content

WIP: enable typescript@next path mapping #283

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

Closed
wants to merge 2 commits into from

Conversation

laurelnaiad
Copy link

This isn't really a pull request, in that I know my "solution" is far from ready for prime time, but I thought I'd throw this up here in case anyone wants to take the ball and run with it toward a real implementation.

See Path mappings based module resolution.

This is a hack to make it possible to use the baseUrl property
in typescript@next. The paths object property probably also works, too.
Less confident about the rootDirs array property.

I believe that baseUrl is to be interpreted as from the directory
where tsconfig.json lives, and this isn't generally avaialable in
gulp-typescript's API, so one reason (among many) that this is a
hack and not a solution is that it assumes the directory found to
be the 'commonBasePath' is assumed to be the tsconfig file's
directory, which may not always be the case.

This is a hack to make it possible to use the `baseUrl` property
in typescript@next. The `paths` object property may also work, too.
Less confident about the `rootDirs` array property.

I believe that baseUrl is to be interpreted as from the directory
where tsconfig.json lives, and this isn't generally avaialable in
gulp-typescript's API, so one reason (among many) that this is a
hack and not a solution is that it assumes the directory found to
be the 'commonBasePath' is assumed to be the tsconfig file's
directory, which may not always be the case.
@ivogabe
Copy link
Owner

ivogabe commented Feb 13, 2016

I'm not sure how this fixes these issues. If the only problem is that baseUrl is relative to the location of tsconfig.json, that could be easily fixed, since the location of that file is known in ts.createProject. Do you think that would fix all issues with the new module resolution?

@laurelnaiad
Copy link
Author

I'm slightly suspicious of the shortcut I put in to interpret non-absolute paths so blithely. I don't even know the spec for these properties down to the bone, so it's hard for me to know exactly what I'm looking for. I'm using baseUrl, and it seems to work, although I have wondered whether there's an issue between how systemjs and Typescript are dealing with the "default js extensions" issue (this would not be the first time this cropped up between the two).

I had tried the paths property when I made a patch for atom-typescript to allow these properties. In atom-typescript's case, paths and baseUrl seemed to "just work".

I haven't tested the logic for paths under gulp-typescript, but if I understand the spec and what is coming out of tsc correctly, then paths should be working here.

I think rootDirs might need extra work from the loader to interpret those rootDirs, but I have not even looked at the specifics.

Sure -- there's a million ways to get the tsconfig path -- if there is one. I don't know what the compiler does or will do if someone uses these as flags without a project....

So, to make a long story short, I think this will probably be a WIP until those issues are worked out. I have limited time to follow through, but I will poke at this as time permits if nobody else does.

Oh, and the code style of my hack should not stand. I think I wrote it as I did to make extra clear that I didn't think it was anywhere near ready for merging. :)

@laurelnaiad
Copy link
Author

Heck -- I suspect this wouldn't even work in windows!

   // "C:\" would fail to be recognized as an absolute path? 
    if (/^\//.test(name)) {

@laurelnaiad
Copy link
Author

I just realized I didn't address your first sentence: "I'm not sure how this fixes these issues."

What this does is check whether Typescript is asking for a rooted/absolute path vs. non-rooted path. If tsc wants the traditional rooted/absolute path, it proceeds as it's always done. If it wants a relative path, the relative path it gives will be based on the resolution of the tsconfig dir and baseUrl (if present). tsc handles that resolution, so the path it asks for needs to be interpreted as from the tsconfig dir. So, we resolve that (using an inaccurate proxy for tsconfig dir) and check if the full path is in the dictionary.

rootDirs I just never tackled, but I imagine some path resolutions need to take place as additional possible file locations.

The issue I have with tsconfigdir is that (a) it's only available way up at the project level, and would need to be passed down deeply into this code and (b) it's not always going to be available, and I don't know what's right to do then...

@laurelnaiad
Copy link
Author

For the record, as of microsoft/TypeScript#7108, there's an additional problem with typescrypt@next in gulp-typescript that's bigger than path mapping support... I will patch this PR for the changes if the Typescript team seems to be intent on renaming the diagnostics property -- can't quite tell, yet. (the change was reverted)

@ivogabe
Copy link
Owner

ivogabe commented Apr 8, 2016

I've taken a look at this and it appeared that setting the baseUrl and rootDirs to absolute paths was enough to make this work (fb4ffc0). Thus, it looks that this PR is not needed any more, but I still want to thank you for your time and effort! You can use the path mappings in the next version, which I plan to release this weekend.

@ivogabe ivogabe closed this Apr 8, 2016
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

Successfully merging this pull request may close these issues.

2 participants