Skip to content

use ds25 provided TS configuration and extend from it #127

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 4, 2025
Merged

use ds25 provided TS configuration and extend from it #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73b90ad
use ds25 provided TS configuration and extend from it
akbarkz Jan 31, 2025
2ac85ba
remove dynamic import as the linter works without it just fine
akbarkz Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions auto-imports.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// @ts-nocheck
// noinspection JSUnusedGlobalSymbols
// Generated by unplugin-auto-import
// biome-ignore lint: disable
export {}
declare global {
const Link: typeof import('react-router-dom')['Link']
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"zustand": "4.5.6"
},
"devDependencies": {
"@canonical/typescript-config-react": "0.4.0-experimental.0",
"@playwright/test": "1.50.1",
"@testing-library/jest-dom": "5.16.5",
"@testing-library/react": "14.0.0",
Expand Down Expand Up @@ -82,4 +83,4 @@
"optionalDependencies": {
"@rollup/rollup-linux-x64-gnu": "4.34.1"
}
}
}
2 changes: 1 addition & 1 deletion static/client/main.test.tsx
View file
Open in desktop
Copy link

@advl advl Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Curious which prop made this change necessary
cc @jmuzina

If this is standard can we capture it in our docs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@advl Yes, this change was caused by this error from TSC:

error TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './main.js'?

There was an option of fixing by adding an extension to the file ./main.tsx and adding additional configuration to tsconfig.json allowImportingTsExtensions. Another option was the one I applied.

Copy link

@advl advl Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can you try "import("./main.js")) and tell me if it works ?

This is due to two things :

  • Using NodeNext, ts expects paths to be fully specified (Also, NodeNext also can be used in the browser, we choose it because it's stricter than EsNext)
  • Without allowImportingTsExtensions the extension should be .js even if the file is .ts or .tsx (typescript is able to resolve that)

The mixture of the two implies that a best practice is to import "my/local/file.js" :)

Reference here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@advl The lint passes if I do it like that as well, although in the IDE it's highlighted with the error Module './main.js' was resolved to '/home/akbar/projects/canonical/websites-content-system/static/client/main.tsx', but '--jsx' is not set.. Shall we go with that then?

Copy link
Member

Choose a reason for hiding this comment

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

There may be something wrong with your IDE here. I cloned this PR and edited this file to use an import with main.js and was able to run yarn build, yarn test, and yarn lint with no errors. Maybe you need to clean-install dependencies?

/* eslint-disable testing-library/no-node-access */
import { act } from "react";

import { waitFor } from "@testing-library/react";

describe("main", () => {
  beforeAll(() => {
    const rootElement = document.createElement("div");
    rootElement.setAttribute("id", "root");
    document.body.appendChild(rootElement);
  });

  it("renders the app in the root element", async () => {
    await act(() => import("./main.js"));
    const container = document.getElementById("root") as HTMLElement;
    await waitFor(() => expect(container.querySelector(".l-application")).toBeInTheDocument());
  });
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@advl @jmuzina indeed, removing all deps and installing again did the trick! removed the dynamic import.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("main", () => {
});

it("renders the app in the root element", async () => {
await act(() => import("./main"));
await act(() => import("./main.js"));
const container = document.getElementById("root") as HTMLElement;
await waitFor(() => expect(container.querySelector(".l-application")).toBeInTheDocument());
});
Expand Down
16 changes: 2 additions & 14 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,10 @@
"compilerOptions": {
"baseUrl": "static/client",
"paths": { "@/*": ["./*"] },
"target": "ESNext",
"useDefineForClassFields": true,
"lib": ["DOM", "DOM.Iterable", "ESNext"],
"allowJs": false,
"skipLibCheck": true,
"esModuleInterop": false,
"allowSyntheticDefaultImports": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"module": "ESNext",
"moduleResolution": "Node",
"resolveJsonModule": true,
"isolatedModules": true,
"noEmit": true,
"jsx": "react-jsx"
"skipLibCheck": true
},
"extends": "@canonical/typescript-config-react",
"include": ["static/client"],
"exclude": ["dist", "public", "node_modules"],
"references": [{ "path": "./tsconfig.node.json" }]
Expand Down
Loading