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.

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

fix: build updated babel-type helpers #17010

Merged
merged 4 commits into from
Dec 6, 2024
Merged

fix: build updated babel-type helpers #17010

merged 4 commits into from
Dec 6, 2024

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Dec 5, 2024 •
edited
Loading

Q                       A
Fixed Issues? Fixes #17006, also fixes the failing CI in #17008
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

In this PR we run the build-rollup step after @babel/types helpers are generated from the definitions. This is not a build issue for Babel 7 because the generated files are checked in the repo. However, in Babel 8, since we started bundling @babel/types in #14179, the @babel/types are now built by the build-rollup step, so we have to run it after the source changes.

In the second commit we fixed another issue that might be related to vinyl-fs: When we run gulp build-dev, we will run the following sequential task: build-no-bundle, generate-type-helpers, build-no-bundle. After generate-type-helpers, the vinyl-fs modified the ctime of e.g. packages/babel-types/src/asserts/generated/index.ts to a much earlier time, therefore it evades the needCompile check in the babel-worker (

babel/babel-worker.cjs

Lines 7 to 20 in 55f9fb5

function needCompile(src, dest) {
let destStat;
try {
destStat = statSync(dest);
} catch (err) {
if (err.code === "ENOENT") {
return true;
} else {
throw err;
}
}
const srcStat = statSync(src);
return srcStat.mtimeMs >= destStat.mtimeMs;
}
) and the developing build never really pick up the types updates. I haven't figure out what is exactly going wrong in vinyl-fs, but we can just replace them with plain fs calls, which works as expected.

@JLHwung JLHwung added the PR: Bug Fix (next major) 🐛 A type of pull request used for our changelog categories for next major release label Dec 5, 2024
@babel-bot
Copy link
Collaborator

babel-bot commented Dec 5, 2024 •
edited
Loading

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58435

@JLHwung JLHwung changed the title fix: run build-rollup after helpers are generated fix: build updated babel-type helpers Dec 6, 2024
@JLHwung JLHwung mentioned this pull request Dec 6, 2024
Merged
nicolo-ribaudo
nicolo-ribaudo approved these changes Dec 6, 2024
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Good investigation, I wonder if maybe it's something related to timezones.

@nicolo-ribaudo nicolo-ribaudo merged commit e9dc2bb into babel:main Dec 6, 2024
55 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the fix-babel-types-builder branch December 6, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liuxingbaoyu liuxingbaoyu liuxingbaoyu approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees
No one assigned
Labels
PR: Bug Fix (next major) 🐛 A type of pull request used for our changelog categories for next major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: path.isTSClassImplements is not a function
4 participants
@JLHwung @babel-bot @nicolo-ribaudo @liuxingbaoyu