Skip to content

feat: allow to import TS files with js extension#3536

Merged
chenjiahan merged 2 commits intomainfrom
import_ts_files_0923
Sep 23, 2024
Merged

feat: allow to import TS files with js extension#3536
chenjiahan merged 2 commits intomainfrom
import_ts_files_0923

Conversation

@chenjiahan
Copy link
Copy Markdown
Member

@chenjiahan chenjiahan commented Sep 23, 2024 •
edited
Loading

Summary

Allow to import TS files with .js extension, align the default behavior with TypeScript.

Related Links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@netlify
Copy link
Copy Markdown

netlify bot commented Sep 23, 2024 •
edited
Loading

✅ Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit 66ba1b2
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/66f14abdad03840008250993
😎 Deploy Preview https://deploy-preview-3536--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 75 (🟢 up 3 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@chenjiahan chenjiahan merged commit 880d337 into main Sep 23, 2024
@chenjiahan chenjiahan deleted the import_ts_files_0923 branch September 23, 2024 13:13
@9aoy 9aoy mentioned this pull request Sep 24, 2024
Merged
colinaaa
colinaaa reviewed Sep 24, 2024
View reviewed changes
packages/core/src/plugins/resolve.ts
if (tsconfigPath) {
// TypeScript allows importing TS files with `.js` extension
chain.resolve.extensionAlias.merge({
'.js': ['.ts', '.tsx', '.js'],
Copy link
Copy Markdown
Contributor

@colinaaa colinaaa Sep 24, 2024

Choose a reason for hiding this comment

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

Should we also add '.jsx': ['.jsx', '.tsx'] as the default extensionAlias?

Copy link
Copy Markdown
Member Author

@chenjiahan chenjiahan Sep 24, 2024

Choose a reason for hiding this comment

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

Agree, we can add '.jsx': ['.tsx', '.jsx'] to align with TypeScript. TS resolves .tsx first.

@joesaunderson
Copy link
Copy Markdown

joesaunderson commented Sep 24, 2024

@chenjiahan did you consider NPM repos that are called <thing>.js here?

Interestingly, on our CI, for 1.0.6 we are seeing failures importing fuse.js and tippy.js - usage:

import Fuse from "fuse.js";

Error

#23 31.41 error   Compile error: 
#23 31.41 Failed to compile, check the errors for troubleshooting.
#23 31.41 File: /.../useFuzzySearch/useFuzzySearch.ts:1:1
#23 31.41   × All of the aliased extensions are not found for /.../src/fuse.js

@chenjiahan
Copy link
Copy Markdown
Member Author

chenjiahan commented Sep 25, 2024

Might be a bug of Rspack resolver, @SoonIter can you help to check it?

@chenjiahan
Copy link
Copy Markdown
Member Author

chenjiahan commented Sep 25, 2024

@joesaunderson This error only appears when fuse.js module is not found, maybe you forget to install it?

image

@SoonIter SoonIter mentioned this pull request Sep 25, 2024
Closed
@joesaunderson
Copy link
Copy Markdown

joesaunderson commented Sep 25, 2024

@joesaunderson This error only appears when fuse.js module is not found, maybe you forget to install it?

image

Hmm not sure, this was working fine on 1.0.5, but for a Dependabot change, where nothing else changes other than 1.0.6, it fails.

It was also failing for 'tippy.js" which makes me think it's a bug, not a failure to install?

I see an issue has been created, does that mean you can now replicate this?

@chenjiahan
Copy link
Copy Markdown
Member Author

chenjiahan commented Sep 25, 2024

No, the issue is about improving the error logs. If you can provide a reproduction, that would be very helpful.

@joesaunderson
Copy link
Copy Markdown

joesaunderson commented Sep 25, 2024

I can replicate with just this minimal config:

Config:

{
    plugins: [pluginReact()],
    source: {
        entry: {
            app: {
                import: path.join(FRONTEND_PATH, "entry.tsx"),
                runtime: "runtime-app",
            },
        },
        tsconfigPath: path.join(BASE_PATH, "tsconfig.json"),
    },
}

TSConfig:

{
    "compilerOptions": {
        "strict": true,
        "baseUrl": "./",
        "paths": {
            "*": ["frontend/src/*"]
        },
        "esModuleInterop": true,
        "jsx": "preserve",
        "isolatedModules": true,
        "module": "esnext",
        "moduleResolution": "node",
        "removeComments": false,
        "incremental": false,
        "sourceMap": false,
        "skipLibCheck": true,
        "target": "esnext",
        "forceConsistentCasingInFileNames": true,
        "noEmit": true,
        "types": ["node", "@types/jest-axe"]
    },
    "include": ["src/**/*", "frontend/**/*"],
    "exclude": ["node_modules"]
}

File (frontend/src/useThing.ts)

import Fuse from "fuse.js";

console.log(Fuse);

Error:

File: /<>/<>/<>/frontend/src/useThing.ts:1:1
  × All of the aliased extensions are not found for /<>/<>/<>/frontend/node_modules/fuse.js

@chenjiahan
Copy link
Copy Markdown
Member Author

chenjiahan commented Sep 25, 2024

@SoonIter you can help to check ⬆️

chenjiahan added a commit that referenced this pull request Sep 25, 2024
@chenjiahan
Revert "feat: allow to import TS files with js extension (#3536)"
dc9812e 
This reverts commit 880d337.
@chenjiahan chenjiahan mentioned this pull request Sep 25, 2024
Merged
2 tasks
chenjiahan added a commit that referenced this pull request Sep 25, 2024
@chenjiahan
Revert "feat: allow to import TS files with js extension (#3536)" (#3559
e3e6cb5 
)
@chenjiahan chenjiahan mentioned this pull request Sep 25, 2024
Merged
@joesaunderson
Copy link
Copy Markdown

joesaunderson commented Sep 25, 2024

@chenjiahan I can confirm that on 1.0.7 this is now working, so your changes above (the revert) fixed that. Thanks.

@renovate renovate bot mentioned this pull request Oct 1, 2024
Merged
1 task
This was referenced Nov 5, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@colinaaa colinaaa colinaaa left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release: feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chenjiahan @joesaunderson @colinaaa