Skip to content

fix: compiler.webpack.ModuleFilenameHelpers type error#4650

Merged
hardfist merged 1 commit intomainfrom
jerry/type
Nov 15, 2023
Merged

fix: compiler.webpack.ModuleFilenameHelpers type error#4650
hardfist merged 1 commit intomainfrom
jerry/type

Conversation

@jerrykingxyz
Copy link
Copy Markdown
Contributor

@jerrykingxyz jerrykingxyz commented Nov 15, 2023 •
edited by hardfist
Loading

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
Copy Markdown
Contributor Author

jerrykingxyz commented Nov 15, 2023

!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
Copy Markdown

rspack-bot commented Nov 15, 2023 •
edited
Loading

📝 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
View reviewed changes
xiaoxiangmoe
xiaoxiangmoe reviewed Nov 15, 2023
View reviewed changes
@jerrykingxyz
Copy link
Copy Markdown
Contributor Author

jerrykingxyz commented Nov 15, 2023

!eco-ci

@rspack-bot
Copy link
Copy Markdown

rspack-bot commented Nov 15, 2023 •
edited
Loading

📝 Ran ecosystem CI: Open

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

@hardfist
Copy link
Copy Markdown
Contributor

hardfist commented Nov 15, 2023 •
edited
Loading

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

@hardfist
Copy link
Copy Markdown
Contributor

hardfist commented Nov 15, 2023

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
@hardfist hardfist deleted the jerry/type branch November 15, 2023 06:18
@hardfist hardfist mentioned this pull request Nov 15, 2023
Closed
Boshen pushed a commit that referenced this pull request Nov 15, 2023
@jerrykingxyz @Boshen
fix: compiler.webpack.ModuleFilenameHelpers type error (#4650)
7f0f825
@hardfist hardfist mentioned this pull request Jan 29, 2024
Merged
2 tasks
@SoonIter SoonIter mentioned this pull request Jul 18, 2024
Closed
2 tasks
@SoonIter
Copy link
Copy Markdown
Member

SoonIter commented Jul 18, 2024

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

@jerrykingxyz
Copy link
Copy Markdown
Contributor Author

jerrykingxyz commented Jul 19, 2024

@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
Copy Markdown
Member

SoonIter commented Jul 19, 2024

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

Reviewers

@hardfist hardfist hardfist approved these changes

@LingyuCoder LingyuCoder Awaiting requested review from LingyuCoder

@ahabhgk ahabhgk Awaiting requested review from ahabhgk

+1 more reviewer

@xiaoxiangmoe xiaoxiangmoe xiaoxiangmoe left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jerrykingxyz jerrykingxyz

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jerrykingxyz @rspack-bot @hardfist @SoonIter @xiaoxiangmoe