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

Typings not being resolved for webpack.config.ts #143

Open
fkleuver opened this issue Apr 14, 2018 · 12 comments
Open

Typings not being resolved for webpack.config.ts #143

fkleuver opened this issue Apr 14, 2018 · 12 comments

Comments

@fkleuver
Copy link
Member

When configuring webpack with a webpack.config.ts rather than a webpack.config.js, the typings are not properly resolved:

image

Everything works, it's just this error and the lack of typings/intellisense that makes the experience suboptimal.

I wouldn't mind submitting a PR if this is indeed a "bug", but I just want to make sure first that there's not some kind of setting that I'm missing.

@Alexander-Taran
Copy link

try adding
"types":"dist/types/index.d.ts"
to plugins package.json first
http://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html
if it will work.. it will be a small PR (-:

@fkleuver
Copy link
Member Author

Bulls-eye, that did the trick!
A simple PR for a change :-)
Cheers!

fkleuver added a commit to fkleuver/webpack-plugin that referenced this issue Apr 14, 2018
@jods4
Copy link
Contributor

jods4 commented Apr 16, 2018

Typings were removed because of #118

I keep this issue open as a reminder to add them back, but it is unlikely to happen before s-panferov/awesome-typescript-loader#484 is fixed.

@fkleuver
Copy link
Member Author

fkleuver commented Apr 16, 2018

@jods4 I switched to ts-loader because at-loader was giving me a variety of issues integrating with non-default stuff. #118 just goes to show that it's indeed a quirky tool.

In the end I noticed no difference in speed. It seems ts-loader has caught up (learned their lesson, maybe?).

Regardless of that, I'm not sure why removing the typings was the better solution here. As far as I know any typing errors caused by external dependencies can be suppressed with skipLibCheck: true. I have this option on by default because I couldn't care less about the absolute correctness of external lib typings as long as they serve documentation needs (they're too often conflicting anyway).

That's an easy workaround whereas there is (apparently?) no workaround for getting the typings included when they're absent.

@jods4
Copy link
Contributor

jods4 commented Apr 16, 2018

Forcing skipLibCheck on users is bad.

These definitions apply to your build file, which is less important than your full app.

The bug with at-loader is that those definitions somehow get included into the full app build, although nothing in app code references aurelia-webpack-plugin.

There are workarounds on the build side as well:

  • /// <reference types="..." /> should work?
  • You can copy the .d.ts file into your build folder.

@fkleuver
Copy link
Member Author

Forcing skipLibCheck on users is bad.

These definitions apply to your build file, which is less important than your full app.

webpack.config.ts is compiled by ts-node and the tsconfig.json for that is usually separate anyway. You're only forcing it on that specific config. The "bad" stays on the build side.

The bug with at-loader is that those definitions somehow get included into the full app build, although nothing in app code references aurelia-webpack-plugin.

I tested it just now and awesome-typescript-loader works just fine with types added to package.json and skipLibCheck: false. Maybe it's been resolved?

/// should work?

That gives me this error: [ts] Cannot find type definition file for './node_modules/aurelia-webpack-plugin/dist/types/index.d.ts'.

Using path instead of types doesn't work either; it simply has no effect (I did reload after changing).

You can copy the .d.ts file into your build folder.

I suppose, but that feels worse than having to set skipLibCheck: true for the ts-node specific tsconfig. There's already so much involved with getting everything type-checked, rather not have to manually keep an external .d.ts in sync on top of that..

@jods4
Copy link
Contributor

jods4 commented Apr 16, 2018

I don't like this situation either.

The top priority is that starting a new app with Webpack should "just work".
At the time, at-loader was popular (I think it even was in some skeletons/templates, maybe even our own) and that scenario would fail.

Fewer people write their build in TS so this is comparatively less important.
It's important that you can make it work, but if there has to be a workaround it'll be on the build-side, not the app-side.

I'm open to any suggestion or new idea that would make the situation better.

I tested it just now and awesome-typescript-loader works just fine with types added to package.json and skipLibCheck: false. Maybe it's been resolved?

Maybe it's been resolved. If we can confirm that the problem was fixed I support bringing back the types in package.json 100%.

@fkleuver
Copy link
Member Author

I tested it with a clean au new with typescript+webpack, and I understand now why I didn't get the issue in my own project.

The tsconfig.json is the real problem. All aurelia ts projects and skeletons seem to have "filesGlob" even though that's not supported. Maybe it once was (and that would be way back), but pretty sure it's simply ignored now.

So this tsconfig:

  "exclude": [
    "node_modules",
    "aurelia_project"
  ],
  "filesGlob": [
    "./src/**/*.ts",
    "./test/**/*.ts",
    "./custom_typings/**/*.d.ts"
  ],

Is essentially saying "include everything except aurelia_project and node_modules".

That includes top-level project files such as webpack.config.ts.

Change it either to this:

  "exclude": [
    "node_modules",
    "aurelia_project",
    "./*.ts"
  ]

Or to this:

  "include": [
    "./src/**/*.ts",
    "./test/**/*.ts"
  ]

And webpack.config.ts is no longer included -> error is gone.

For clarification, there's 3 properties pertaining to input: "include", "exclude" and "files"

  • "include" does what "filesGlob" is thought to do, by whoever authored these configs
  • "include" can contain globs, "files" cannot
  • If "include" and "files" are omitted, everything (except the "exclude" glob) ending with .ts is included
  • "exclude" takes priority over "include", "files" takes priority over "exclude"

In all my projects I only ever specify "include". I think this is the cleanest solution. Only ["src"] for the build, and ["src", "test"] for testing. That's all you need in 99% of cases.

I sort of knew this all along but I figured there must have been something I didn't know about "filesGlob". I triple checked the docs now, and I'm 100% sure it must be replaced with "include" in all aurelia projects (and checked for correctness) if we want everything to be consistent on the long term.

@Alexander-Taran
Copy link

Alexander-Taran commented Apr 16, 2018

@fkleuver that will result in tests being compiled and given to webpack.. always gives me headaches..
I also like them to be next to models as *.test.ts
for at-loader there could be files:[] but it errors out for ts-loader.. and seems to have strange sideeffects..

nice catch though. We should figure out tsconfig(s) for every cli/skeleton check them with both loaders.. and all cli included testers..

then we can proceed. I think

@fkleuver
Copy link
Member Author

@Alexander-Taran That's what's currently happening anyway. The example I gave changes as little as possible to the existing configuration. Removing "tests" from the glob might break stuff if we're having an "include" to begin with.

We should figure out tsconfig(s) for every cli/skeleton check them with both loaders.. and all cli included testers..

It should be quite simple as they're all very similar. I just learned that "filesGlob" is specific to the atom editor. It was never part of typescript itself.

So to non-atom users it never did anything. "includes" will behave the way "filesGlob" was intended to.

For atom users, nothing should change since "includes" does the same thing in typescript as "filesGlob" does in atom.

This would also eliminate inconsistencies between non-atom users and atom users (@EisenbergEffect ? ;)) in how things are included/compiled.

@EisenbergEffect
Copy link
Contributor

I agree. We want this to be as consistent as possible. @AshleyGrant can you look into updated the skeletons based on @fkleuver's PR to the CLI?

@thomas-darling
Copy link

Can we please either include the typings for aurelia-webpack-plugin in the package, or alternatively publish them as @types/aurelia-webpack-plugin?

Missing typings really get in the way, and having to work around it by adding your own .d.ts file is pretty annoying - especially when its the last dependency in your project that needs one of those ;-)

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 a pull request may close this issue.

5 participants