Closed moltar closed 11 months ago
ErikCH
commented
2 years ago Hi @moltar !
Thanks for bringing this up! Yes, we have strict: false in our React library right now. This would be a good fix for the future. We did fix our Vue and Angular libraries, and strict: true is on for those. We'll add this to our future back log!
ErikCH
commented
2 years ago Also, I'm not sure how tied this is into our JS library. It will take more research
ErikCH
commented
2 years ago Can you also try this in your tsconfig as well?
"compilerOptions": {
"skipLibCheck": true,
If you're still stuck that is. Otherwise turning strict off could help too.
moltar
commented
2 years ago @ErikCH I have not tried that, but others have reported this not making a difference.
But I also don't think this is a path forward. Ultimately the upstream library has to be fixed.
Why would we undermine the type checking for the entire project, just because of one small bug that can be fixed easily?
reesscot
commented
2 years ago Hi @moltar,
We will be looking into enabling strict: true on the ui-react package in this issue: https://github.com/aws-amplify/amplify-ui/issues/1109
In the meantime, please give the option @ErikCH mentioned a try. This is actually the default setting in Create React App projects and will only disable strict type checking for libraries not your own code (which I realize still isn't ideal). You can try out my TS CRA example here and see that it compiles without any errors with the "skipLibCheck" option enabled while leaving "strict" on for the rest of the project.
reesscot
commented
2 years ago @moltar Feel free to reopen if adding "skipLibCheck": true doesn't fix the errors.
moltar
commented
2 years ago @moltar Feel free to reopen if adding
"skipLibCheck": truedoesn't fix the errors.
I fundamentally disagree with the premise of skipLibCheck option, and I will not undermine the integrity of our very large and complex application by disabling this important option just to fix an issue in a UI component.
This is actually the default setting in Create React App projects
That does not mean it is correct.
Our application consists of many things that go beyond CRA, in fact the React app is a tiny fragment of it. Inlcuding AWS CDK, backend code, utility functions. And all of that is in one single project.
reesscot
commented
1 year ago Just to provide an update on this. Some code in ui-react has moved to ui-react-core, and during the process we enabled TS strict mode. In addition the react-native package has TS strict mode enabled, and we will ensure any new packages are strict. We are planning to completely enable strict mode on the main react package as well early next year.
reesscot
commented
1 year ago Doing another pass at reproducing this issue.
First created a TS Vite3 app:
npm create vite@latest ui-react-test --template react-ts
cd ui-react-test
npm install @aws-amplify/ui-react@latest aws-amplifyThen disabled skipLibCheck in my tsconfig.json file:
{
"compilerOptions": {
"target": "ESNext",
"useDefineForClassFields": true,
"lib": ["DOM", "DOM.Iterable", "ESNext"],
"allowJs": false,
"skipLibCheck": false, // This is the key change
"esModuleInterop": false,
"allowSyntheticDefaultImports": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"module": "ESNext",
"moduleResolution": "Node",
"resolveJsonModule": true,
"isolatedModules": true,
"noEmit": true,
"jsx": "react-jsx"
},
"include": ["src"],
"references": [{ "path": "./tsconfig.node.json" }]
}
Add the following to App.tsx:
import "./App.css";
import { Authenticator, AccountSettings } from "@aws-amplify/ui-react";
import { Amplify } from "aws-amplify";
Amplify.configure({});
const formFields = {
signIn: {
username: {
placeholder: "Enter Your Email Here",
isRequired: true,
label: "Email:",
},
},
};
export default function App() {
return (
<Authenticator formFields={formFields}>
{({ signOut }) => {
const handleSuccess = () => {
alert("password is successfully changed!");
};
return <AccountSettings.ChangePassword onSuccess={handleSuccess} />;
}}
</Authenticator>
);
}
Running npm run build I get the following output:
➜ cra-ts-strict git:(master) ✗ npx tsc --noEmit
node_modules/@aws-amplify/ui-react/dist/types/components/Geo/LocationSearch/index.d.ts:2:37 - error TS2307: Cannot find module 'maplibre-gl-geocoder' or its corresponding type declarations.
2 import { LocationSearchProps } from 'maplibre-gl-geocoder';
~~~~~~~~~~~~~~~~~~~~~~
node_modules/@aws-amplify/ui/dist/types/utils/index.d.ts:5:36 - error TS7016: Could not find a declaration file for module 'lodash/isEqual.js'. '/Users/reesscot/cra-ts-strict/node_modules/lodash/isEqual.js' implicitly has an 'any' type.
Try `npm i --save-dev @types/lodash` if it exists or add a new declaration (.d.ts) file containing `declare module 'lodash/isEqual.js';`
5 export { default as isEqual } from 'lodash/isEqual.js';
~~~~~~~~~~~~~~~~~~~
node_modules/@aws-amplify/ui/dist/types/utils/index.d.ts:6:36 - error TS7016: Could not find a declaration file for module 'lodash/isEmpty.js'. '/Users/reesscot/cra-ts-strict/node_modules/lodash/isEmpty.js' implicitly has an 'any' type.
Try `npm i --save-dev @types/lodash` if it exists or add a new declaration (.d.ts) file containing `declare module 'lodash/isEmpty.js';`
6 export { default as isEmpty } from 'lodash/isEmpty.js';
~~~~~~~~~~~~~~~~~~~
.... a bunch more lodash related errors
node_modules/type-fest/ts41/get.d.ts:93:37 - error TS2344: Type 'BaseType' does not satisfy the constraint 'Record<string | number, any>'.
93 : Key extends keyof WithStringKeys<BaseType>
~~~~~~~~
node_modules/type-fest/ts41/get.d.ts:79:17
79 type PropertyOf<BaseType, Key extends string> =
~~~~~~~~
This type parameter might need an `extends Record<string | number, any>` constraint.
node_modules/type-fest/ts41/get.d.ts:94:19 - error TS2344: Type 'BaseType' does not satisfy the constraint 'Record<string | number, any>'.
94 ? WithStringKeys<BaseType>[Key]
~~~~~~~~
node_modules/type-fest/ts41/get.d.ts:79:17
79 type PropertyOf<BaseType, Key extends string> =
~~~~~~~~
This type parameter might need an `extends Record<string | number, any>` constraint.
Found 13 errors in 3 files.
Errors Files
1 node_modules/@aws-amplify/ui-react/dist/types/components/Geo/LocationSearch/index.d.ts:2
10 node_modules/@aws-amplify/ui/dist/types/utils/index.d.ts:5
2 node_modules/type-fest/ts41/get.d.ts:93We've addressed these errors here: https://github.com/aws-amplify/amplify-ui/pull/3471 https://github.com/aws-amplify/amplify-ui/pull/3472 https://github.com/aws-amplify/amplify-ui/pull/3473
Now after installing via next tag, the errors go away.
npm install @aws-amplify/ui-react@next
npm run build
// no errorsWe will get the fix release for the errors reproduced above. We are continuing to make progress on towards full TS Strict support.
reesscot
commented
1 year ago @moltar We have made a number of TS Strict improvements and while we aren't completely there, I was wondering if you would be willing to see if you're still getting the errors above.
moltar
commented
1 year ago @reesscot Thanks for the follow-up. Any specific version I should be looking at?
reesscot
commented
1 year ago @moltar I was thinking if you wouldn't mind testing the latest tag.
Also thought anyone following this thread would be interested in commenting on this Amplify JS API RFC for TypeScript support: https://github.com/aws-amplify/amplify-js/issues/11113
moltar
commented
1 year ago Hey @reesscot I can confirm that the originally reported problems are gone.
A new problem had appeared though:
node_modules/amazon-cognito-identity-js/index.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: UpdateAttributesNodeCallback, ClientMetadata, AuthenticationDetails, ChallengeName, CognitoUser, CognitoUserAttribute, CognitoUserPool, CognitoUserSession, CognitoAccessToken, CognitoIdToken, CognitoRefreshToken, CookieStorage, UserAgent, appendToCognitoUserAgent, WordArray
1 declare module 'amazon-cognito-identity-js' {
~~~~~~~
node_modules/@aws-amplify/auth/node_modules/amazon-cognito-identity-js/index.d.ts:1:1
1 declare module 'amazon-cognito-identity-js' {
~~~~~~~
Conflicts are in this file.
node_modules/amazon-cognito-identity-js/index.d.ts:4:14 - error TS2649: Cannot augment module 'NodeCallback' with value exports because it resolves to a non-module entity.
4 export type NodeCallback<E, T> = (err?: E, result?: T) => void;
~~~~~~~~~~~~
node_modules/amazon-cognito-identity-js/index.d.ts:402:3 - error TS2687: All declarations of 'domain' must have identical modifiers.
402 domain: string;
~~~~~~
node_modules/amazon-cognito-identity-js/index.d.ts:402:3 - error TS2717: Subsequent property declarations must have the same type. Property 'domain' must be of type 'string | undefined', but here has type 'string'.
402 domain: string;
~~~~~~
node_modules/@aws-amplify/auth/node_modules/amazon-cognito-identity-js/index.d.ts:402:3
402 domain?: string;
~~~~~~
'domain' was also declared here.❯ npm ls amazon-cognito-identity-js
├─┬ @aws-amplify/auth@5.2.0
│ └── amazon-cognito-identity-js@6.2.0
└─┬ @aws-amplify/ui-react@4.4.1
└─┬ aws-amplify@5.0.11
├─┬ @aws-amplify/api@5.0.11
│ └─┬ @aws-amplify/api-graphql@3.0.11
│ └─┬ @aws-amplify/auth@5.1.5
│ └── amazon-cognito-identity-js@6.1.2 deduped
├─┬ @aws-amplify/auth@5.1.5
│ └── amazon-cognito-identity-js@6.1.2
├─┬ @aws-amplify/datastore@4.0.11
│ ├─┬ @aws-amplify/auth@5.1.5
│ │ └── amazon-cognito-identity-js@6.1.2 deduped
│ └── amazon-cognito-identity-js@6.1.2 deduped
└─┬ @aws-amplify/pubsub@5.0.11
└─┬ @aws-amplify/auth@5.1.5
└── amazon-cognito-identity-js@6.1.2 dedupedThanks for checking @moltar! I'm transferring this issue over the the amplify-js repo now, since the new errors aren't coming from the ui packages.
cwomack
commented
1 year ago The developer preview for v6 of Amplify has officially been released with improvements to TypeScript support and much more! Please check out our announcement and updated documentation to see what has changed.
This issue should be resolved within the dev preview and upcoming General Availability for Amplify v6, but let us know with a comment if there are further issues.
cwomack
commented
11 months ago With the release of the latest major version of Amplify (aws-amplify@>6), this issue should now be resolved! Please refer to our release announcement, migration guide, and documentation for more information.
New Problem:
Original Issue
Before creating a new issue, please confirm:
On which framework/platform are you having an issue?
React
Which UI component?
Other
Please describe your bug.
When running TS build on the project, I am getting lots of errors:
What's the expected behaviour?
No errors.
Help us reproduce the bug!
Use strict TS. Use your package.
Code Snippet
N/A
Additional information and screenshots
Related issue: https://github.com/aws-amplify/amplify-js/issues/7188