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

Set up i18n package #6991

Merged
merged 16 commits into from
Sep 13, 2024
Merged

Set up i18n package #6991

merged 16 commits into from
Sep 13, 2024

Conversation

araujogui
Copy link
Member

Description

Set up i18n package

Validation

ESLint is throwing Unable to resolve path to module '@node-core/website-i18n' error and I couldn't figure out the fix. Some help is welcome

Related Issues

Related to #5405

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run format to ensure the code follows the style guide.
  • I have run npm run test to check if all tests are passing.
  • I have run npx turbo build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

@araujogui araujogui requested review from a team as code owners August 19, 2024 01:16
Copy link

vercel bot commented Aug 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Sep 13, 2024 1:04pm

Copy link

Note

Your Pull Request seems to be updating Translations of the Node.js Website.

Whilst we appreciate your intent; Any Translation update should be done through our Crowdin Project.
We recommend giving a read on our Translation Guidelines.

Thank you!

@bmuenzenmeyer
Copy link
Collaborator

bmuenzenmeyer commented Aug 19, 2024

will take a look at this as i can - also #6993 might conflict

@bmuenzenmeyer
Copy link
Collaborator

I can recreate
I've tried adding "dependsOn": ["@node-core/website-i18n#build"], according to https://turbo.build/repo/docs/crafting-your-repository/configuring-tasks#depending-on-a-specific-task-in-a-specific-package - but no luck yet

@ovflowd
Copy link
Member

ovflowd commented Aug 23, 2024

I can recreate I've tried adding "dependsOn": ["@node-core/website-i18n#build"], according to turbo.build/repo/docs/crafting-your-repository/configuring-tasks#depending-on-a-specific-task-in-a-specific-package - but no luck yet

Can we ask help for to our in-house Turborepo specialist on Slack?

@bmuenzenmeyer
Copy link
Collaborator

@anthonyshew curious if you'd have some time to look at this and help out? the lint step is failing on the branch

@anthonyshew
Copy link
Contributor

This is going to end up being a misconfiguration somewhere on ESLint or typescript-eslint, but it's going to be a fun time trying to figure out where it is. 😄 That's at least my early hypothesis since the application appears to be building correctly, so module resolution is doing what it's supposed to.

Digging in...

packages/i18n/package.json Outdated Show resolved Hide resolved
packages/i18n/.eslintrc.json Outdated Show resolved Hide resolved
apps/site/i18n.tsx Show resolved Hide resolved
@anthonyshew
Copy link
Contributor

After I couldn't sort it out, I asked Josh for his help. In typical Josh fashion, he overdelivered.

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Sep 8, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Sep 8, 2024
@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Sep 9, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Sep 9, 2024
@ovflowd
Copy link
Member

ovflowd commented Sep 9, 2024

I've rebased the PR, hopefully this is fixed now!

@ovflowd
Copy link
Member

ovflowd commented Sep 9, 2024

cc @JoshuaKGoldberg, it seems like tests are failing due to being unable to import the module; I assume it's because Jest is not using TypeScript's module resolution here. (note that our website uses a mix of TypeScript and plain ESM, So ESM should also be able to resolve said modules)

My assumption here is that the Jest config needs to be updated to understand the module resolution for the said module. (Just a 5 minute guess, after looking on the CI logs)

@ovflowd
Copy link
Member

ovflowd commented Sep 9, 2024

There are many issues here regarding module resolution during tests and build. This definitely needs to be addressed.

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Sep 9, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Sep 9, 2024
@ovflowd
Copy link
Member

ovflowd commented Sep 9, 2024

To be honest, the ESlint v8 setup is really flaky, we are using some weird canary versions of plugins, and I'm really uncomfortably with how things are 🫠

This added a hella of instability here.

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Sep 13, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Sep 13, 2024
@bmuenzenmeyer bmuenzenmeyer added this pull request to the merge queue Sep 13, 2024
Merged via the queue into nodejs:main with commit b266643 Sep 13, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants