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

--config should not apply to deps #7701

Closed
Skillz4Killz opened this issue Sep 26, 2020 · 8 comments
Closed

--config should not apply to deps #7701

Skillz4Killz opened this issue Sep 26, 2020 · 8 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@Skillz4Killz
Copy link

--configs tsconfig.json should not apply to dependencies only my code. I can't control other libraries code and it makes it near impossible to rely on every dependency using the same configurations as my personal choices

image

image

I can no longer use my project because a dependency of dependency does not comply with these settings. For now, I just removed these options but the point I am hoping to convey is that --configs is not really useable in the real world. Now, these are just simple options that might not break most projects but what happens when 4.1 becomes enabled and I add the noUncheckedIndexedAccess compiler option to my tsconfig? How many modules will become unusable for me?

Dependency modules should not be compiled using the custom compiler options.

Relevant Issue: #5460

Alternative: --no-check I really don't think this option is a good option to solve this as this would remove all the benefits of using TS and tsconfig in the first place since it also disables checking of my code.

@kitsonk kitsonk added suggestion suggestions for new features (yet to be agreed) cli related to cli/ dir labels Sep 26, 2020
@kitsonk
Copy link
Contributor

kitsonk commented Sep 26, 2020

I don't think we want to support this. The complexity versus value is too great, even if it was achievable.

In the specific example you've given, those items are much better suited for linting purposes.

Ultimately, in fact, supporting --config is a challenge at all. There really should only be one way to do TypeScript under Deno. Variations only cause heartache and problems. For the purposes of --no-emit there is only a handful of options that we should ultimately support, and the rest should be disabled, in my opinion. We are currently too permissive. Those options are:

  • checkJs
  • emitDecoratorMetadata
  • jsx
  • jsxFactory
  • jsxFragmentFactory

This would imply that experimentalDecorators is true by default (it already effectively is with --no-check) and while I would still strongly discourage their use, it is not a challenge for Deno to support the legacy TypeScript decorators. Everything else shouldn't be open for debate in Deno, as it only fragments the community and makes code potentially incompatible.

@ry, @bartlomieju thoughts?

@Skillz4Killz
Copy link
Author

Skillz4Killz commented Sep 26, 2020

@kitsonk What about other options? What happens when 4.1 and I want this noUncheckedIndexedAccess enabled in my code? It is absolutely crucial for proper type-safety. Will Deno lint handle this option as well?

You recommended me to use the --config option to be able to use this once #7573 is merged, as this isn't something you would enable. This option is too crucial to have and is the reason I began testing compiler options. If this option is enabled in my config how many Deno modules will become unusable for me since my project's tsconfig has a stricter requirement.

Should any project's configs be applied to others whose code we can not control(deps)?


That said I absolutely love the idea of getting rid of configurations. I actually hate configs/options. However, I do believe that if this is done, Deno should adopt the strictest options possible to provide the best type-safety that TS has to offer because otherwise, Deno would only be causing me to lose benefits that TS can give.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 26, 2020

The TypeScript core team specifically do not recommend enabling noUncheckedIndexedAccess by default because it has too many edge cases.

My feeling is we follow --strict for code that Deno runs, which contains the agreed generally appropriate language semantics. No stronger or weaker opinions then that.

@Skillz4Killz
Copy link
Author

Skillz4Killz commented Sep 26, 2020

@kitsonk IMO you can indeed choose to avoid enabling options so long as you have some way to allow users to enable that option to have type safety? If you remove the --configs as you mentioned above then that becomes impossible for me to enable making me lose TS safety.

And with the current --configs, I still can't use it because if I do enable it'll break 90% of modules out there so Deno itself becomes pretty bad to use for me. If I want more type safety, I can't use Deno. That's not a good thing.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 27, 2020

You Keep Using That Word, I Do Not Think It Means What You Think It Means

The example you current give has nothing to do with type safety. They are stylistic improvements, ones which deno lint already covers. And again, noUnchechedIndexedAccess improves type safety in some instances but is categorically wrong in lots of other real world legitimate ones. Just because we might not support a single option doesn't mean it burns down the whole benefit of TypeScript, especially for one option that never existed in TypeScript until a current beta release.

The benefits of an opinionated set of options means everyone know how they can write TypeScript code for Deno, and how to validate, instead of having to try to figure out what the maximum set of options that need to be set are that don't conflict. It is a huge and growing problem with the wider TypeScript eco-system, something I think we need to make the responsible choice and not add to.

@Skillz4Killz
Copy link
Author

With all due respect, I do know what it means. I have already applied the noUnchechedIndexedAccess across my projects and gained massive fixes and safety improvements. Yes, I have also seen a few downsides but I believe this option is worth it. If it is removed, we lose access to increased type-safety which yes is a concern!

image

I did not say that I lose all the benefits of TS if you don't allow a single option. But I am losing some TS benefits in my project without this option. If I were to use Deno, I would not be able to get faster performance, fewer rate limits, and bandwidth being wasted as well as preventing bugs in runtime which cause hours of debugging because you have to go line by line since TS says everything is good!


But all that is beside the point. This isn't about 1 or 2 specific options but the general decision of how --configs works. Using Deno, should never make it less type-safe than using another runtime. I encourage removing --configs but opting for the strictest configs. If --configs is kept then it needs to be reworked as applying my tsconfig to every dep doesn't really work either.

As I said above, I 100% support the opinionated set kit. I love less configs. However, I don't think the opinionated set should be limited to --strict only. But that's a separate discussion of which options to keep if --configs is removed. I don't want to get sidetracked here.

The main issue is that tsconfigs -configs are applied to every dep, to which the possible solutions are:

  • Remove --configs for an opinionated set of tsconfigs applied to all Deno projects
  • --configs be applied only to your own code and not other deps.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 28, 2020

Issue #7732 is opened about removing --config all together.

Again, to be clear, as for applying different configs to different sets of TypeScript files, I am opposed to it.

@bartlomieju
Copy link
Member

@kitsonk I believe there's definitely no way we could implement that now and as mentioned #7732 proposes to remove --config flag together so I'm going to respectfully decline and close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

3 participants