ionic-team / ionic-app-scripts

App Build Scripts for Ionic Projects
http://ionicframework.com/
MIT License
608 stars 304 forks source link

Setting BaseUrl in tsconfig.json #678

Open brandyscarney opened 7 years ago

brandyscarney commented 7 years ago

From @shyamal890 on January 23, 2017 10:22

Ionic version: (check one with "x") [ ] 1.x [x] 2.x

I'm submitting a ... (check one with "x") [x] bug report

I am trying to set BaseUrl to './src' in tsconfig.json. Though it works well with typescript, it is not working and breaking the app on ionic serve

Copied from original issue: driftyco/ionic#10142

danbucholtz commented 7 years ago

@shyamal890,

We don't support this option. We are open to a well-tested PR that implements it, though.

What is the use case for this options?

Thanks, Dan

shyamal890 commented 7 years ago

@danbucholtz I find absolute url more enterprise friendly than relative urls as in a large app, as time passes things move around and this would break the import logic due to relative urls. Whereas in case of absolute urls only few urls would have to be changed.

danbucholtz commented 7 years ago

Sounds good. It's something we'll consider long term but it's not on our roadmap for the next several months.

Thanks, Dan

danbucholtz commented 7 years ago

We decided this isn't on our roadmap for the next several months, so we're going to close this and revisit at a later date.

Thanks, Dan

pacebaby commented 7 years ago

Cannot understand why you make this decision. Please think about the comercial, large project. It's is better using the non-relative path than the relative path. on the other side, it dose not cost too much to implement this.

danbucholtz commented 7 years ago

If enough people want this feature, we'll implement eventually. It's just not on our roadmap for the next few months. We are cracking down on issues. We are a small team and it is nearly impossible to stay on top of them. Our new policy is if we're not doing it soon, we close it by default.

Thanks, Dan

playerx commented 7 years ago

+1 this feature

playerx commented 7 years ago

https://github.com/driftyco/ionic-app-scripts/issues/125 https://github.com/driftyco/ionic-app-scripts/issues/149

deliriousrhino commented 7 years ago

+1 please support

glebinsky commented 7 years ago

really need this feature

danbucholtz commented 7 years ago

We would be open to a PR. What is the use case for this feature? Hundreds of thousands of people are building apps without it.

Thanks, Dan

glebinsky commented 7 years ago

@danbucholtz There are 2 cases where relative paths become a nightmare:

  1. Non flat file structure where referencing dependencies requires you to do this ../../../../../some_module/some_sub_module/provider/file.ts. It becomes a guessing game of how many ../ I need depending on where I am and what I'm referencing

  2. Having to move the example module or sub-module to another location and changing all those ../../...... I spent hours updating references because global s/p can't handle every case.

I'm sure y'all have tons of higher priority issues ahead of this, but dropping this feature - which is already available in Angular 2 - will alienate anyone building a large complex mobile app.

danbucholtz commented 7 years ago

To some extent I think if you are going back that many levels, you're over-engineering the directory structure. I feel like it could definitely be simpler even for a large app. Sorry, we have no plans to add this at this time. We'd be open to a well-tested PR.

If you have a declarations.d.ts file with a * in it, you could remove that and tsc will immediately tell you about broken imports. the declarations.d.ts file was a relic of yesteryear to account for some of TypeScript's short comings with 3rd party libs.

Thanks, Dan

playerx commented 7 years ago

@danbucholtz,

What is the use case for this feature? Hundreds of thousands of people are building apps without it.

It's much simple and elegant way to use: import {...} from 'services';

in any place of your app, rather then import {...} from '../../services'; import {...} from '../services';

same for pages and so on...

P.S. Hundreds of thousands of people build apps on Ionic because it has cool features and not because it doesn't support of BaseUrl. It was incorrect argument sir.

P.P.S. People already use this feature in Angular and they love it (so am I), that's why so many people comment here and want this feature.

P.P.P.S. Please keep in mind what community wants and it will make Ionic better and better

P.P.P.P.S. I ❤️ Ionic 👍

josh-sachs commented 7 years ago

All other arguments aside, you are basically throwing out a subset of Typescript by not supporting this convention. The limitation appears to be in the lack of extensibility of the module loader (webpack).

To some extent I think if you are going back that many levels, you're over-engineering the directory structure.

I fucking hate when people use these kinds of statements to gloss over issues. Each module in an angular application is it's own concern. Why should a file in one module give a shit where it exists relative to a file in another module? Not immediately realizing how important it is to abstract the file structure out of an application - to me - means the wrong person is managing this entire project.

shyamal890 commented 7 years ago

Frankly, this issue should be reopened till the time the feature is not implemented.

danbucholtz commented 7 years ago

@josh-sachs,

That sort of language will not be tolerated. This is your one and only warning. Please review our code of conduct before commenting again.

@playerx, @shyamal890,

We're not opposed to the feature at all. We would love to have the feature! It is just too low of priority in a sea of high priority items to get to. We can take the time to add this, which affects a tiny, tiny, tiny subset of apps, or we can work on improving start-up time, speeding up builds, improving performance, fixing navigation bugs, etc. These things affect hundreds of thousands of apps, versus a handful of thumbs up here. Believe me, we know what the highest priority items are, and we're focused on them.

If you wish to take a crack at it with a PR, by all means, go ahead. I am happy to help you get started and answer any questions you may have. My name on Ionic slack if danb. Feel free to msg me.

Thanks, Dan

davidmpaz commented 7 years ago

I know this have been long discussed. Still, these are my 5 cents:

I am sorry this tiny feature is not supported or planned to be supported in the short term. I belong to that tiny subset of guys using it extensively :-) in combination of package authoring/importing.

hope it is done sooner that later

Cheers

danbucholtz commented 7 years ago

We're going to reopen this. We would be opened to a PR with some unit tests. My DMs are always open and I can help out. This would be a medium hardness PR I think, but I can help.

Thanks, Dan @

funkjedi commented 7 years ago

It's not a PR but an easy solution for those in need is to just use a custom webpack.config.js.

var path = require('path')
var tsconfig = require('../tsconfig.json')
var webpackConfig = require('@ionic/app-scripts/config/webpack.config');

webpackConfig.dev.resolve = webpackConfig.prod.resolve = {
  extensions: ['.ts', '.js', '.json'],
  modules: [path.resolve('node_modules'), path.resolve(tsconfig.compilerOptions.baseUrl)]
}

module.exports = webpackConfig

Update your package.json to reference the custom config and you're good to go.

    "config": {
        "ionic_webpack": "./scripts/webpack.config.js"
    }
vellengs commented 7 years ago

should better use a plugin

var path = require('path') 
var webpackConfig = require('@ionic/app-scripts/config/webpack.config');

const {
  TsConfigPathsPlugin
} = require('awesome-typescript-loader');

webpackConfig.dev.resolve = webpackConfig.prod.resolve = {
  extensions: ['.ts', '.js', '.json'],
  modules: [path.resolve('node_modules')],
  plugins: [
    new TsConfigPathsPlugin( /* { configFileName, compiler } */ )
  ]
}
LloydVincent commented 7 years ago

my two cents: I used ionic 2/3 for a large-scale real world app for a over year and a half. Using index.ts buckets helped keep the imports somewhat orderly, but I still cringed every time I had to figure out the correct number of ".." segments needed when I added a new component. Refactored folder structure a couple times and that was a headache too. It's not life-or-death but this feature would be really appreciated.

ViniciusAzevedo commented 6 years ago

+1 this feature [2]

Needing this just now, for a multi-modular app!

MattiLehtinen commented 6 years ago

Would appreciate this feature too. With absolute paths large scale apps would be much more maintainable.

brandonmarino commented 6 years ago

@funkjedi

I wanted path aliases working too so I modified your solution a little with some help from here: https://robferguson.org/blog/2017/11/22/working-with-typescript-webpack-and-ionic-3/

My webpack.config.js looks like this now:

var path = require('path');
var useDefaultConfig = require('@ionic/app-scripts/config/webpack.config.js');
var tsconfig = require('../tsconfig.json');

let aliases = {};

let pathKeyArray = Object.keys(tsconfig.compilerOptions.paths);
pathKeyArray.forEach(currentPath => {
    let correctPath = currentPath.replace("/*","");
    let currentPathValue = tsconfig.compilerOptions.paths[currentPath][0].replace('*','');
    aliases[correctPath] = path.resolve(tsconfig.compilerOptions.baseUrl+'/'+currentPathValue);
})

useDefaultConfig.dev.resolve = useDefaultConfig.prod.resolve = {
    extensions: ['.ts', '.js', '.json'],
    modules: [
        path.resolve('node_modules'),
        path.resolve(tsconfig.compilerOptions.baseUrl)
    ],
    alias:aliases
}

module.exports = function () {
  return useDefaultConfig;
};

Inside my tsconfig.json

{
  "compilerOptions": {
    "baseUrl": "./src",
    "paths": {
        "@path1/*": [
          "path1/*"
        ],
        "@path2/*": [
          "path2/*"
        ]
    }
  ...
}

This works properly for me now. Which is great, because I needed this for a shared component library. Thanks 👍

HeavenSwordz commented 6 years ago

+1

FanFanFantastic commented 5 years ago

@funkjedi @brandonmarino Thank you guys so much and it worked. This thread has cancer, it has that dictatorship feeling in it.

bastien612 commented 5 years ago

+1 for an official release of this feature. It's way easier to be able to use import from a base URL for refacto and code readability. All major languages support this feature (actually, even TypeScript does). That really sad to see it broken in Ionic.

bastien612 commented 5 years ago

Does anyone has problem with the configuration added by funkjedi ? Because it seems to work pretty well for me.