Core: Add multiple DefinePlugin support for process.env #13949
Conversation
|
@phated I added lodash only because there are already several packages in storybook that use lodash as a dependency. Thit is But if you insist, I can refactor it |
There was a problem hiding this comment.
What exactly does this PR do? Storybook strips out the contents of process.env because if we don't it's a massive security problem since the contents get bundled into your webpage and served out to the entire internet. If you are only using your Storybook on your dev machine or on a secured network, or on "safe" machines without any sensitive environment variables set, I could imagine an opt-in "dangerous" mode. But never as Storybook's default behavior.
|
@shilman it doesn't leak variables. Let me explain: Our source content: // src.js
console.log(process.env.BAR)
console.log(process.env.FOO)The current behaviour of storybook close to this: new webpack.DefinePlugin({
'process.env': {
bar: 'bar_value',
// some others value
}
}),// dist.js
console.log(({"bar":bar_value}).BAR)
console.log(({"bar":bar_value}).FOO)With this PR not defined env variables will be untouched: new webpack.DefinePlugin({
'process.env.BAR': 'bar_value',
// some others value
}),// dist.js
console.log(bar_value)
console.log(process.env.FOO)So it makes possible use several DefinePlugins Before this PR: new webpack.DefinePlugin({
'process.env': {
bar: 'bar_value',
// some others value
}
}),
new webpack.DefinePlugin({
'process.env': 'some_global_variable_that_stores_values'
}),// dist.js
console.log(({"bar":bar_value}).BAR)
console.log(({"bar":bar_value}).FOO)After this PR: new webpack.DefinePlugin({
'process.env.BAR': 'bar_value'
}),
new webpack.DefinePlugin({
'process.env': 'some_global_variable_that_stores_values'
}),// dist.js
console.log(bar_value)
console.log(some_global_variable_that_stores_values.FOO)And now I can set variables in runtime by modyfing |
|
@7rulnik Thanks so much for the clarification!! 🙏 |
shilman
changed the title
# Conflicts: # lib/builder-webpack4/src/preview/iframe-webpack.config.ts # lib/core-server/src/manager/manager-webpack.config.ts
|
@7rulnik some refactors happened in the storybook core, and I want to help out bring this PR back in shape, but I do not have access to push to it, could you enable this option? |
|
@ndelangen I can't allow edits by maintainers because it's a repo in the organization, not mine, I guess. So I sent an invite to the repo for you. Also, I'm not sure that #14203 does the job. Why do you think it's breaking change? |
@7rulnik because people might have enumerated over
What makes you think that? Perhaps we should add an option so users can choose between replacing: vs vs That way replacing |
Problem
Current storybook behavior is overriding the whole
process.env. I'm trying to use it with https://github.com/7rulnik/isomorphic-env-webpack-plugin and it doesn't work becauseprocess.envalready replaced by the storybook. So for now it's possible to findDefinePlugininwebpackFinaland replacedefinitionsby hand.What I did
Currently
DefinePluginhas definiton like this:After this PR
DefinePluginwill have:How to test
Currently, we have a custom snapshot serializer that outputs only plugins names.
But it should possible to set any env variable like before and override some env that doesn't set by pushing
DefinePlugininwebpackFinal.