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

Report errors in JS(X) files when CheckJS is enabled #564

Merged
merged 1 commit into from Jun 25, 2017

Conversation

schmuli
Copy link
Contributor

@schmuli schmuli commented Jun 25, 2017

Closes #518

@schmuli
Copy link
Contributor Author

schmuli commented Jun 25, 2017

@johnnyreilly This is the PR, as discussed. I wasn't able to run the tests locally, because of Corporate proxy settings, and some tests are failing. Additionally, I didn't add any tests for this change.

@johnnyreilly
Copy link
Member

Thanks! I think the test failure is just a flaky test (they're full integration tests and so we get the odd false negative) Just kicked off a test run again.

@johnnyreilly johnnyreilly merged commit bcbb3ce into TypeStrong:master Jun 25, 2017
@johnnyreilly
Copy link
Member

Great! Thanks for this - the fix will go out with the next release 👍

@schmuli schmuli deleted the fix-checkjs-errors branch June 26, 2017 08:17
@OliverJAsh
Copy link

I'm noticing that this is reporting errors from JS files inside node_modules. Is that expected? I.e. I get a lot more errors running via ts-loader than I do when running via tsc.

@johnnyreilly
Copy link
Member

Are you running with 2.2.2? Should be resolved now

@OliverJAsh
Copy link

I'm using version 2.4.1. Example of errors:

ERROR in ./~/react-dom/lib/ReactDOM.js
(43,3): error TS2304: Cannot find name '__REACT_DEVTOOLS_GLOBAL_HOOK__'.

ERROR in ./~/react-dom/lib/ReactDOM.js
(68,16): error TS2304: Cannot find name '__REACT_DEVTOOLS_GLOBAL_HOOK__'.

ERROR in ./~/react-dom/lib/ReactDOM.js
(82,40): error TS2551: Property 'documentMode' does not exist on type 'Document'. Did you mean 'DOCUMENT_NODE'?

@OliverJAsh
Copy link

Ah sorry, yes, ts-loader 2.2.2, tsc 2.4.1

@johnnyreilly
Copy link
Member

Also are you excluding node_modules in your webpack.config.js?

@OliverJAsh
Copy link

OliverJAsh commented Jul 3, 2017

@johnnyreilly Yes, I believe so:

{
  module: {
    rules: [
      {
        test: /\.ts$/,
        exclude: /node_modules/,
        use: [
          {
            loader: 'babel-loader',
          },
          {
            loader: 'ts-loader',
            options: {
              entryFileIsJs: true,
            }
          },
        ],
      },
    ]
  }
}

@johnnyreilly
Copy link
Member

Hmmmm may be an issue then. If you lose the entryFileIsJs option does the problem resolve? (You will need to change the entry file too)

@schmuli
Copy link
Contributor Author

schmuli commented Jul 3, 2017

You can try the --skipLibCheck option but I haven't checked that this changes anything.

On the other hand, I am using 2.2.2 with a big mixed project, including lots of JS in node_modules and I haven't seen this issue yet, so this is probably a configuration issue. What does your tsconfig.json look like?

@OliverJAsh
Copy link

Have you enabled checkJs on that project?

@OliverJAsh
Copy link

@schmuli @johnnyreilly I spent awhile narrowing this issue down, smallest reproduction case here #577

@johnnyreilly
Copy link
Member

Thanks for the repro - we appreciate it! I wonder, could you test with ts-loader v2.2.0 please? This was the version prior to @schmuli's change. Does the issue arise with that version?

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 this pull request may close these issues.

None yet

3 participants