Skip to content

[WIP] Suffix all imports with .js #1491

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

Jump to bottom
Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

[WIP] Suffix all imports with .js #1491

wants to merge 6 commits into from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented May 23, 2025 •
edited
Loading

  • Identify all packages that need to be updated to add the .js suffix to imports
  • Create a script to scan and update imports in all .ts files
  • Test the script on a small set of files to verify it works correctly
  • Run the script on all packages
  • Verify build works with npx tsc
  • Run tests to ensure everything functions correctly after the changes

Fixes #1001.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@julien-c
Copy link
Member

sounds good, let's go!

@julien-c
Copy link
Member

can you try again?

@Copilot Copilot SWE Agent
Copy link
Contributor Author

Copilot AI commented May 26, 2025

can you try again?

I've created and run a script that adds .js suffix to all relative imports in TypeScript files. I also fixed some specific import issues in the packages/hub and packages/agents packages to ensure they build correctly. The changes have been tested on multiple packages (agents, jinja, inference) and they all build successfully now. The latest commit is d7074c4.

Copilot finished work on behalf of julien-c May 26, 2025 09:54
julien-c
julien-c reviewed May 26, 2025
View reviewed changes
packages/agents/src/HfAgent.ts
Copy link
Member

Choose a reason for hiding this comment

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

do NOT change the quote to single quotes

Copy link
Member

Choose a reason for hiding this comment

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

in any file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed the quote style back to double quotes while maintaining the correct import path. The changes can be seen in commit 2cde019.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed the quote style in all files back to double quotes while maintaining the correct .js suffix in imports. This change is in commit 52a781d.

Copy link
Member

Choose a reason for hiding this comment

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

lol @julien-c can probably just run the formatter to fix it

Copy link
Member

Choose a reason for hiding this comment

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

i don't have a local computer anymore :/

julien-c
julien-c reviewed May 26, 2025
View reviewed changes
tsconfig.json
Copy link
Member

Choose a reason for hiding this comment

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

don't commit this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the tsconfig.json changes in commit 52a781d.

julien-c
julien-c reviewed May 26, 2025
View reviewed changes
scripts/add-js-suffix.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

don't commit this in the final version of the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the add-js-suffix.ts script from the PR in commit 52a781d.

Copilot finished work on behalf of julien-c May 26, 2025 15:28
@Copilot Copilot SWE Agent
Copy link
Contributor Author

Copilot AI commented May 26, 2025 •
edited
Loading

can you try again?

I've updated the PR to fix several issues:

  1. Changed all import quote styles back to double quotes
  2. Removed the tsconfig.json changes
  3. Removed the add-js-suffix.ts script

The changes are in commit 52a781d.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • hg.mozilla.org
    • Triggering command: node ./dist/install.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Copilot @julien-c
I'll analyze the diff and prepare a commit message.
ae2f381 
<think>
Let me understand what I've been working on based on the PR description and commits:

1. The PR is about adding `.js` suffix to all imports in TypeScript files
2. I've created and run a script to add `.js` suffixes to relative imports
3. I fixed specific import issues in packages/hub and packages/agents
4. There were issues with quote styles (changing double quotes to single quotes) that needed to be reverted
5. I'm supposed to remove some temporary files (scripts/add-js-suffix.ts and tsconfig.json changes)

The most recent commits:
1. "Fix quote style and remove unnecessary files"
2. "Fix quote style in HfAgent.ts imports"
3. "Fix module imports with proper .js suffixes"
4. "Add suffix to imports and fix build issues"

So the overall task is to ensure all imports have `.js` suffix for TypeScript files.
</think>

Update TypeScript imports to include .js suffix in filenames

This commit updates the import statements in TypeScript files to include
.js extensions in the file paths, ensuring compatibility with ES modules.
The changes maintain the project's code style with double quotes for
import statements.

Fixes #1001.

Co-authored-by: julien-c <[email protected]>
Copilot finished work on behalf of julien-c May 26, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julien-c julien-c julien-c left review comments

@coyotte508 coyotte508 coyotte508 left review comments

@ngxson ngxson Awaiting requested review from ngxson ngxson will be requested when the pull request is marked ready for review ngxson is a code owner

@enzostvs enzostvs Awaiting requested review from enzostvs enzostvs will be requested when the pull request is marked ready for review enzostvs is a code owner

@mishig25 mishig25 Awaiting requested review from mishig25 mishig25 will be requested when the pull request is marked ready for review mishig25 is a code owner

@xenova xenova Awaiting requested review from xenova xenova will be requested when the pull request is marked ready for review xenova is a code owner

@nsarrazin nsarrazin Awaiting requested review from nsarrazin nsarrazin will be requested when the pull request is marked ready for review nsarrazin is a code owner

@hanouticelina hanouticelina Awaiting requested review from hanouticelina hanouticelina will be requested when the pull request is marked ready for review hanouticelina is a code owner

@SBrandeis SBrandeis Awaiting requested review from SBrandeis SBrandeis will be requested when the pull request is marked ready for review SBrandeis is a code owner

Assignees

@julien-c julien-c

Copilot code review Copilot

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suffix all imports with .js
3 participants
@Copilot @julien-c @coyotte508