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

add platform target properties to compiler. #18378

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vankop
Copy link
Member

@vankop vankop commented May 1, 2024

What kind of change does this PR introduce?

add 2 methods compiler.getPlatformTargetInfo and compiler.setPlatformTargetInfo to allow get and set platform target properties. by default resolve this properties from target option.

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?
compiler.getPlatformTargetInfo and compiler.setPlatformTargetInfo. Also new PlatformPlugin that allow to change platform options, e.g. new PlatformPlugin({ node: true })

@webpack-bot
Copy link
Contributor

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)

add 2 methods `compiler.getPlatformTargetInfo` and `compiler.setPlatformTargetInfo` to allow get and set platform target properties. by default resolve this properties from target option.
@vankop vankop force-pushed the feat-platform-target-properties-in-compiler branch from b83e17e to e11fb12 Compare May 1, 2024 14:28
Copy link
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.

Looks good, but let's apply this things for loader context too, I don't want to use hacky _compiler in loaders to get these things, for example css-loader can export only locals when you your target is node

@vankop
Copy link
Member Author

vankop commented May 7, 2024

could we add this in another PR? @alexander-akait

node: targetProperties.node,
nwjs: targetProperties.nwjs,
electron: targetProperties.electron
};
Copy link
Member

Choose a reason for hiding this comment

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

Let's return all targetProperties and set them here https://github.com/webpack/webpack/pull/18378/files#diff-7979ef8b26e853797876ef65fffd2669a8c088f6ae7853028ed4b876f9f0be65R85, I imagine that in the future we may need more properties of our targets

Copy link
Member Author

Choose a reason for hiding this comment

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

rest of the target properties related to environment, they are accessible in output.environment

Copy link
Member Author

Choose a reason for hiding this comment

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

@alexander-akait
Copy link
Member

@vankop Let's rebase

# Conflicts:
#	lib/config/defaults.js
#	lib/webpack.js
#	types.d.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants