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

Package do not mesh well with TSC and allowJs #437

Open
kant2002 opened this issue Feb 25, 2021 · 15 comments
Open

Package do not mesh well with TSC and allowJs #437

kant2002 opened this issue Feb 25, 2021 · 15 comments
Assignees
Labels

Comments

@kant2002
Copy link

Package Version: 2.2.3

To Reproduce
Steps to reproduce the behavior:

  1. npm init
  2. npm install @azure/ms-rest-js ts-loader webpack webpack-cli
  3. Create index.ts
  4. Create webpack.config.js
  5. Create tsconfig.json
  6. Run npx webpack

index.js

import * as msRest from "@azure/ms-rest-js";

console.log(msRest);

webpack.config.js*

const webpack = require('webpack');
const bundleOutputDir = './dist';

module.exports = (env) => {
    return [{
        entry: {
            'index': './index',
        },
        resolve: {
            extensions: ['.ts']
        },
        output: {
            path: path.join(__dirname, bundleOutputDir),
            filename: '[name].js',
        },
        module: {
            rules: [
                {
                    test: /\.js|.ts?$/, loader: 'ts-loader'
                }
            ],
        },
    }];
};

tsconfig.json

Make sure that allowJs set to true

{
    "compilerOptions": {
      "moduleResolution": "node",
      "target": "ES2019",
      "strict": true,
      "module": "commonjs",
      "allowJs": true, 
    },
    "exclude": [
      "node_modules",
      "dist",
      "webpack.config.js"
    ]
  }

package.json

{
  "name": "msrestjs",
  "version": "1.0.0",
  "main": "index.ts",
  "scripts": {
    "test": "webpack"
  },
  "dependencies": {
    "@azure/ms-rest-js": "2.2.3",
    "ts-loader": "8.0.17",
    "webpack": "5.24.2",
    "webpack-cli": "4.5.0"
  }
}

** Error output **

ERROR in ...\node_modules\@azure\ms-rest-js\es\lib\msRest.js
./node_modules/@azure/ms-rest-js/es/lib/msRest.js 3:21-37
[tsl] ERROR in ...\node_modules\@azure\ms-rest-js\es\lib\msRest.js(3,22)
      TS6053: File '.../node_modules/@azure/ms-rest-js/es/dom-shim.d.ts' not found.

Expected behavior
Compilation without errors

@jeremymeng
Copy link
Member

We have a tripple-slash reference to "../dom-shim.d.ts" in msRest.ts and we pack this file at root folder of the package. The build output dir is es/ but es/lib/msRest.js still keep the reference path as /// <reference path="../dom-shim.d.ts" />. Of course the file isn't there. @witemple-msft I don't know if you've seen this before. Core-http also have the same problem.

@jeremymeng jeremymeng added the bug label Feb 25, 2021
@jeremymeng
Copy link
Member

@witemple-msft does the file dom-shim.d.ts have to be under package root? Could it be along side index.ts so we could use ./dom-shim.d.ts?

@jeremymeng
Copy link
Member

Ok, it does look like Typescript compilation won't copy the reference to the output folder, so probably why we keep it outside of the source tree.

@jeremymeng
Copy link
Member

jeremymeng commented Feb 25, 2021

@kant2002 I tried the repro steps, however I need to do a couple of things before I see the error about dom-shim.d.ts.

  1. hit an error [webpack-cli] ReferenceError: path is not defined. Resolved by adding a require in webpack.config.js
  2. hit an error Error: Cannot find module 'typescript'. Resolved by npm install -D typescript

then I see 28 errors, the last one about dom-shim.d.ts. Are you seeing similar errors? I want to make sure I am not missing something as the errors seem indicating nothing would work for me.

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 7:20-44 Module not found: Error: Can't resolve './webResource' in 'C:\working\rest-js-437\node_modules\@Azure\ms-rest-js\es\lib' @ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 9:26-56
Module not found: Error: Can't resolve './defaultHttpClient' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 11:20-44
Module not found: Error: Can't resolve './httpHeaders' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 13:29-62
Module not found: Error: Can't resolve './httpPipelineLogLevel' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 15:18-40
Module not found: Error: Can't resolve './restError' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 17:22-48
Module not found: Error: Can't resolve './serviceClient' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 20:30-64
Module not found: Error: Can't resolve './queryCollectionFormat' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 22:18-45
Module not found: Error: Can't resolve './util/constants' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 24:18-49
Module not found: Error: Can't resolve './policies/logPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 26:22-57
Module not found: Error: Can't resolve './policies/requestPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 29:38-89
Module not found: Error: Can't resolve './policies/generateClientRequestIdPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 31:31-75
Module not found: Error: Can't resolve './policies/exponentialRetryPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 33:31-75
Module not found: Error: Can't resolve './policies/systemErrorRetryPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 35:30-73
Module not found: Error: Can't resolve './policies/throttlingRetryPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 37:20-53
Module not found: Error: Can't resolve './policies/agentPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 39:20-53
Module not found: Error: Can't resolve './policies/proxyPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 42:23-59
Module not found: Error: Can't resolve './policies/redirectPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 44:22-57
Module not found: Error: Can't resolve './policies/signingPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 46:24-61
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 49:30-73
Module not found: Error: Can't resolve './policies/deserializationPolicy' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 52:19-42
Module not found: Error: Can't resolve './serializer' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 56:14-37
Module not found: Error: Can't resolve './util/utils' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 69:12-28
Module not found: Error: Can't resolve './url' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 73:25-66
Module not found: Error: Can't resolve './credentials/tokenCredentials' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 75:39-94
Module not found: Error: Can't resolve './credentials/basicAuthenticationCredentials' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 77:26-68
Module not found: Error: Can't resolve './credentials/apiKeyCredentials' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 79:25-66
Module not found: Error: Can't resolve './credentials/topicCredentials' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in ./node_modules/@azure/ms-rest-js/es/lib/msRest.js 81:26-68
Module not found: Error: Can't resolve './credentials/domainCredentials' in 'C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib'
@ ./index.ts 3:15-43

ERROR in C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib\msRest.js
./node_modules/@azure/ms-rest-js/es/lib/msRest.js 3:21-37
[tsl] ERROR in C:\working\rest-js-437\node_modules@azure\ms-rest-js\es\lib\msRest.js(3,22)
TS6053: File 'C:/working/rest-js-437/node_modules/@azure/ms-rest-js/es/dom-shim.d.ts' not found.
@ ./index.ts 3:15-43

@kant2002
Copy link
Author

@jeremymeng sorry for delay. You can checkout minimal repro https://github.com/kant2002/msrestjs-437

@kant2002
Copy link
Author

I have global typescript installed, but likely it can be installed in the project too.

@jeremymeng
Copy link
Member

@kant2002 Thanks for the repro project! I had global typescript (v4.1.2) installed too but still getting the same error even with your project. Could you please help perform the following test?

  • cp node_modules/@azure/ms-rest-js/dom-shim.d.ts node_modules/@azure/ms-rest-js/es/ (*Nix) or copy .\node_modules\@azure\ms-rest-js\dom-shim.d.ts .\node_modules\@azure\ms-rest-js\es\ (Windows)
  • Run npx webpack again

and see if that helps?

I was playing with the repro project, and found that I could get the webpack to bundle the project by

  • installing typescript locally
  • adding '.js' to the resolve extensions in webpack.config.js
resolve: {
            extensions: ['.ts', '.js']
        },
  • copying cp node_modules/@azure/ms-rest-js/dom-shim.d.ts node_modules/@azure/ms-rest-js/es/

@kant2002
Copy link
Author

kant2002 commented Mar 1, 2021

@jeremymeng So essentially copying cp node_modules/@azure/ms-rest-js/dom-shim.d.ts node_modules/@azure/ms-rest-js/es/ is the only options for now?

@jeremymeng
Copy link
Member

@kant2002 It's just an experiment. If it fixed the issue, we could potentially add it in the npm package.

@kant2002
Copy link
Author

kant2002 commented Mar 2, 2021

@jeremymeng copy helps.

@kant2002
Copy link
Author

kant2002 commented Mar 9, 2021

@jeremymeng will this be fixed, or this would be unsupported feature?

@jeremymeng
Copy link
Member

@kant2002 Thanks for the confirmation! We definitely want to fix the issue but also need to prioritize it among work items. Will keep you updated.

@jeremymeng
Copy link
Member

This is an interesting scenario. @bterlson @xirzec @chradek @witemple-msft any thoughts?

Summary

  • in out build output es/lib/msRest.js, we have a triple-slash reference to ./dom-shim.d.ts
  • when using TSC option allowJs is set to true, an error is reported because compiler cannot resolve ./dom-shim.d.ts from es/lib/msRest.js

One potential fix is copying that file to es/lib in the NPM prepack script. Are there any implications?

@xirzec
Copy link
Member

xirzec commented Mar 30, 2021

So the problem was introduced by this change? 773a3f0

I guess it's tricky to fix this without re-introducing the problems of #367

I'm a little confused why we seem to have both

https://github.com/Azure/ms-rest-js/blob/master/lib/dom.d.ts
and
https://github.com/Azure/ms-rest-js/blob/master/dom-shim.d.ts

Can't we just remove dom.d.ts, move dom-shim.d.ts into the lib folder, and everything should still compile? Or do we use more than is shimmed?

@witemple-msft
Copy link
Member

We should probably do the same thing here (and in core-http v1 for track-2) as we did in Azure/azure-sdk-for-js/pull/14092.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants