Skip to content
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

fix(@angular/cli) - include "dom" lib in root tsconfig.json #5060

Merged
merged 1 commit into from
Feb 28, 2017

Conversation

deebloo
Copy link
Contributor

@deebloo deebloo commented Feb 27, 2017

Add "dom" lib to root tsconfig.json. fixes #5046

Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

LGTM, but you have to change the commit message since that test is failing. The other two failing tests are flakes.

@deebloo
Copy link
Contributor Author

deebloo commented Feb 27, 2017

@filipesilva done. ha I manged to make a mistake in a 3 char pr 😆

@deebloo deebloo force-pushed the add-dom-lib branch 3 times, most recently from 39db3b3 to a6178f0 Compare February 27, 2017 19:17
@Meligy
Copy link
Contributor

Meligy commented Feb 28, 2017

Can we also add "target": "es5" to this?

I was planning to open a discussion about it, tested in VS Code, tslint fails things like:

get someProperty() {
  return `some calculated value`;
}

Because it says you can only have getters when targeting ES5 or higher.

That's because it can only read the closest tsconfig.json file. tsconfig.app.json means nothing to it.

@deebloo
Copy link
Contributor Author

deebloo commented Feb 28, 2017

@Meligy should the target be es5 or should we bump it straight to es2016 since by default TS allows you to use that syntax?

@deebloo deebloo force-pushed the add-dom-lib branch 4 times, most recently from 5d7deef to c089d67 Compare February 28, 2017 02:54
@Meligy
Copy link
Contributor

Meligy commented Feb 28, 2017

target is the output, which for the browser (non-node) stuff needs to be es5. We have libs and polyfills for many ES2015 features, but the syntax that cannot be polyfilled needs to be transpiled to ES5.

That's in theory. In reality it doesn't matter, because tsconfig.spec.json and tsconfig.app.json have their own target values (es6 and es5 respectively). This root file is more just for IDEs etc if I get it right.

So, I think we should set it to the minimum of both files, which is es5.

@Meligy
Copy link
Contributor

Meligy commented Feb 28, 2017

To explain it a little further: TypeScript in theory supports targeting es3 (outputting to es3), but each feature in TypeScript is only supported in some targets.

For example, property getters are not supported when targeting es3. async/await in TypeScript 1.x was only supported when targeting es6 not even es5.

If you don't specify anything, the lower value is assumed, which is es3. This is for backwards compatibility,
Just like when you don't specify moduleResolution, it's assumed classic, although everyone is (and should be) using node.

@deebloo
Copy link
Contributor Author

deebloo commented Feb 28, 2017

Makes sense @filipesilva what do you think?

@filipesilva
Copy link
Contributor

Makes sense to me as well.

@deebloo
Copy link
Contributor Author

deebloo commented Feb 28, 2017

updated to add es5 to target

@cexbrayat
Copy link
Member

I'm possibly mistaken, but doesn't that mean that we can remove the lib and target from tsconfig.app.json?

See #5111 for an attempt

@filipesilva
Copy link
Contributor

@cexbrayat that's right, yes. I added a request for changes to your PR, the es5 bit still needs to stay for non-ng4 projects.

asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017
@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.

TS2304: cannot find name "xxx"——after update to rc.0
5 participants