Skip to content

Add project source dir as a module solution #2210

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

abner
Copy link
Contributor

@abner abner commented Sep 19, 2016

Adding resolve alias @prefix and app to webpack config and adding paths @prefix and app to tsconfig.json. I think it fixes #1465

This allows to import app modules using

import '@app/app.component'

or

import 'app/app.component';

@ghost
Copy link

ghost commented Sep 19, 2016

This seems to only work with app/, but what if I want to add another module, like so:

/src/
   login/ # root module for checking auth, otherwise showing a signin/signup page
   app/ # the actual app, only displayed when auth'ed

I think that we should have some kind of wildcard mapping, such that import * from '@something'; will map to /src/something/index.ts. At least for the webpack-part, because it is not configurable for a end-user. I don't think that this kind of wildcard-mapping works with the current tsconfig. What do you think?

Maybe we need a module registry in the angular-cli.json for that, or it will be detected. For example: crawl through each folder x/ in /src/ and check if there's a x.module.ts. If so, x is a module and should be added to the mapping

@k1ng440
Copy link

k1ng440 commented Sep 20, 2016

i have tested bdb1953 and its works

@k1ng440
Copy link

k1ng440 commented Sep 20, 2016

@abner please update your branch

@abner
Copy link
Contributor Author

abner commented Sep 20, 2016

OK @k1ng440 , it is updated now.

@abner
Copy link
Contributor Author

abner commented Sep 20, 2016

There is a plan to merge it soon? Just asking because if is not going to be merged we will change our projects to temporarily reference a local version of angular-cli

"baseUrl": ".",
"paths": {
"@<%= prefix %>/*": ["app/*"],
"app/*": ["app/*"]
Copy link
Member

@clydin clydin Sep 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the blueprint, including just @app might be a good idea to keep things simple. More of a preference thing though, i suppose.

Copy link
Member

@clydin clydin Sep 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, thinking about it a little bit. It might be better to just add support for paths and add defaults in a follow-up PR. Many people that want this will most likely customize anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i will change to just get the paths defined in the tsconfig.json applied to webpack resolve without defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just removed this from the blueprint tsconfig.json

@kylecordes
Copy link

I'm not sure this feature is actually a net good idea. I just wrote a comment about it in a similar item linked below. I'm not sure whether in the overall trade-off this is good or bad, but I think it's worthy of discussion before going in.

#2254

@clydin
Copy link
Member

clydin commented Sep 21, 2016

@kylecordes, I think adding support for typescript's paths option is definitely a good idea. However, as I suggested above in a comment, removing the default path definitions from the blueprint would probably be best for reasons you suggest.
This would allow someone to use the options if they desire (and make their setup as complicated as they wish) but not potentially confuse the new/beginner developer.

@ValeryVS
Copy link
Contributor

Look at examples in #2254
There are @ packages, in node_modules.
Probably, it will be better to choose another prefix symbol for inner modules. Then difference become more clear.

@abner
Copy link
Contributor Author

abner commented Sep 21, 2016

changed the blueprint tsconfig.json to not have any path or baseUrl by default;

@@ -12,11 +12,6 @@
"target": "es5",
"typeRoots": [
"../node_modules/@types"
],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the baseUrl and paths setup was actually removed from the blueprint tsconfig.json, beside the erroneous commit message. 😄

@ghost
Copy link

ghost commented Sep 23, 2016

I'm so happy with this PR. Hope this will be merged soon, because this is the only thing blocking us from upgrading to the webpack version!

@k1ng440
Copy link

k1ng440 commented Sep 23, 2016

@rolandoldengarm me too.

@clydin
Copy link
Member

clydin commented Sep 23, 2016

@abner, you might want to squash commits and clean up the commit message to simplify a review.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@googlebot
Copy link

CLAs look good, thanks!

@filipesilva
Copy link
Contributor

To be clear, this PR simply adds support to the paths property in tsconfig.json, correct?

I remember we had this at a point, but for some reason took it out. Maybe it was broken at the time.

@hansl @TheLarkInn can you review? Both preferably.

@hansl
Copy link
Contributor

hansl commented Sep 27, 2016

We're moving away from using awesome-typescript-loader for various reasons. This kind of path-mapping should be used in the blueprints, really, IMO. I'm going to close this since it is fundamentally incompatible with #2333 .

@filipesilva
Copy link
Contributor

Superseded by #2470.

@applemate
Copy link

@filipesilva I want to able to import like this, what I config before using it? I saw suggestion about this usage everywhere in this repo but nothing official about how to config.

@filipesilva
Copy link
Contributor

@craigcosmo you can use the tsconfig paths property. A good description can be found here http://stackoverflow.com/questions/34925992/how-to-avoid-imports-with-very-long-relative-paths-in-angular-2/34926643

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add project source dir as a module
10 participants