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

fix: compiler.webpack.ModuleFilenameHelpers type error #4650

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

jerrykingxyz
Copy link
Collaborator

@jerrykingxyz jerrykingxyz commented Nov 15, 2023

Summary

Example:
Input:

module.exports  = xxx;

Ouput: (NodeNext)

Object.defineProperty(exports, "__esModule", { value: true }); // this seems a bug of ts compiler, which need further investigation
module.exports = xxx;
// console undefined

Output: (CommonJS)

module.exports = xxx;

Test Plan

Require Documentation?

  • No
  • Yes, the corresponding rspack-website PR is __

@jerrykingxyz jerrykingxyz requested review from a team, LingyuCoder and ahabhgk November 15, 2023 05:15
@jerrykingxyz
Copy link
Collaborator Author

!eco-ci

@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Nov 15, 2023
@rspack-bot
Copy link

rspack-bot commented Nov 15, 2023

📝 Ran ecosystem CI: Open

suite result
modernjs ❌ failure
nx ✅ success
rspress ✅ success
rsbuild ❌ failure
compat ✅ success

hardfist
hardfist previously approved these changes Nov 15, 2023
@jerrykingxyz
Copy link
Collaborator Author

!eco-ci

@rspack-bot
Copy link

rspack-bot commented Nov 15, 2023

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
nx ✅ success
rspress ✅ success
rsbuild ✅ success
compat ✅ success

@hardfist
Copy link
Contributor

hardfist commented Nov 15, 2023

@xiaoxiangmoe it's indeed caused by nodeNext, let revert temporarily and dig into it later

@hardfist
Copy link
Contributor

let's merge it

@hardfist hardfist added this pull request to the merge queue Nov 15, 2023
Merged via the queue into main with commit f32f5be Nov 15, 2023
17 checks passed
@hardfist hardfist deleted the jerry/type branch November 15, 2023 06:18
@SoonIter
Copy link
Member

@jerrykingxyz how can i test whether this bug is fixed in latest Typescript version "5.5.3"
#7211

@jerrykingxyz
Copy link
Collaborator Author

@SoonIter

  1. checkout this PR and replace module to NodeNext
  2. try access compiler.webpack.ModuleFilenameHelper and it will throw a type error
  3. upgrade the ts version and access compiler.webpack.ModuleFilenameHelper again

@SoonIter
Copy link
Member

image

I think this issue is caused by const ModuleFilenameHelpers = exports;

this is a surprising writing of cjs code ... I think it has been fixed by renaming to .ts file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants