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

Address msal-node tsdx warning #2202

Merged
merged 5 commits into from
Aug 28, 2020
Merged

Address msal-node tsdx warning #2202

merged 5 commits into from
Aug 28, 2020

Conversation

tnorling
Copy link
Collaborator

Addresses warning when building msal-node:

[tsdx]: Your rootDir is currently set to "./". Please change your rootDir to "./src". TSDX has deprecated setting tsconfig.compilerOptions.rootDir to "./" as it caused buggy output for declarationMaps and occassionally for type declarations themselves. You may also need to change your include to remove "test", which also caused declarations to be unnecessarily created for test files.

Also hopefully fixes intermittent build error:

EBUSY: resource busy or locked, copyfile 'D:\a\1\s\lib\msal-node\dist\src\utils\NetworkUtils.d.ts' -> 'D:\a\1\s\lib\msal-node\dist\utils\NetworkUtils.d.ts'

@github-actions github-actions bot added the msal-node Related to msal-node package label Aug 27, 2020
import { Constants as NodeConstants } from './../utils/Constants';
import { TokenCache } from '../cache/TokenCache';
import { ClientAssertion } from "../client/ClientAssertion";
const pjson = require("../../package.json");
Copy link
Member

Choose a reason for hiding this comment

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

I thought we are avoiding require except in tests.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Collaborator Author

@tnorling tnorling Aug 27, 2020

Choose a reason for hiding this comment

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

Hmm then we may need to think of something because we can't import package.json from outside the root without using require

Copy link
Contributor

@pkanher617 pkanher617 Aug 28, 2020

Choose a reason for hiding this comment

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

I think it's ok for this scenario. Did some testing and there isn't any way this will work otherwise currently because tsdx has issues with . as the rootDir. This is one reason I don't think we should have used tsdx.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So even adding a @ts-ignore in order to import, silently changes the rootDir back to ./ in the background. We'll move forward with the require for now and revisit later

@coveralls
Copy link

coveralls commented Aug 27, 2020

Coverage Status

Coverage remained the same at 83.23% when pulling 6084dd4 on tsdx-fix into f2cef5a on dev.

Copy link
Member

@hectormmg hectormmg left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

6 participants