Skip to content

add node-module option for node.__file/dirname#14247

Merged
alexander-akait merged 13 commits intomainfrom
feature-14072
Jan 10, 2024
Merged

add node-module option for node.__file/dirname#14247
alexander-akait merged 13 commits intomainfrom
feature-14072

Conversation

@vankop
Copy link
Copy Markdown
Member

@vankop vankop commented Sep 14, 2021

What kind of change does this PR introduce?

feature

closes #14072

Did you add tests for your changes?

yes

Does this PR introduce a breaking change?

no

What needs to be documented once your changes are merged?

new options node.__filename=node-module / node.__dirname=node-module

generate __filename and __dirname for common js modules when output.module=true to
fileURLToPath(import.meta.url) and fileURLToPath(import.meta.url + "/..") respectively

evaluate __filename and __dirname for common js modules when output.module to
fileURLToPath(import.meta.url) and fileURLToPath(import.meta.url + "/..") respectively
@webpack-bot
Copy link
Copy Markdown
Contributor

webpack-bot commented Sep 14, 2021

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

Comment thread test/configCases/output-module/node-globals/index.js Outdated
Comment thread test/configCases/output-module/node-globals/index.js Outdated
Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
@webpack-bot
Copy link
Copy Markdown
Contributor

@vankop Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@webpack-bot
Copy link
Copy Markdown
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

# Conflicts:
#	lib/dependencies/CachedConstDependency.js
#	schemas/WebpackOptions.check.js
Copy link
Copy Markdown
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

Found these old review comments... not sure if they are still relevant

Comment thread lib/dependencies/CachedConstDependency.js Outdated
Comment thread lib/DependencyTemplate.js Outdated
Comment thread test/configCases/output-module/node-globals/index.js Outdated
Comment thread lib/NodeStuffPlugin.js Outdated
Comment thread lib/dependencies/NodeUrlDependency.js Outdated
Comment thread lib/javascript/JavascriptGenerator.js Outdated
@@ -199,7 +199,8 @@ class JavascriptGenerator extends Generator {
runtime: generateContext.runtime,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unrelated: For performance reasons we should only create templateContext once per module.

# Conflicts:
#	schemas/WebpackOptions.check.js
# Conflicts:
#	lib/DependencyTemplate.js
#	lib/javascript/JavascriptGenerator.js
#	schemas/WebpackOptions.check.js
#	types.d.ts
Copy link
Copy Markdown
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

@vankop can you rebase, I think we can merge it and I am fine with it, thank you

@iamyamakin
Copy link
Copy Markdown
Contributor

@sokra do you mind if i create PR with a merge commit? I see a few conflicts between this branch and main:

  • lib/NodeStuffPlugin.js;
  • lib/config/defaults.js;
  • schemas/WebpackOptions.check.js.

@snitin315
Copy link
Copy Markdown
Member

I'll rebase this PR, no worries.

@alexander-akait
Copy link
Copy Markdown
Member

Yeah, feel free to send again, I am fine with the such improve

@webpack-bot
Copy link
Copy Markdown
Contributor

I've created an issue to document this in webpack/webpack.js.org.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Shipped

Development

Successfully merging this pull request may close these issues.

ESM builds should substitute __filename and __dirname references in CommonJS

6 participants