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

diffhtml: process is not defined #268

Closed
justjake opened this issue Apr 13, 2022 · 6 comments · Fixed by #269 or #270
Closed

diffhtml: process is not defined #268

justjake opened this issue Apr 13, 2022 · 6 comments · Fixed by #269 or #270

Comments

@justjake
Copy link
Contributor

The check for process.env breaks unless I use Webpack's DefinePlugin to replace the process.env check in diffhtml's source during build.

Consider first checking if typeof process !== 'undefined').

@tbranyen
Copy link
Owner

tbranyen commented Apr 14, 2022

Do you know where this is happening?

export default typeof process !== 'undefined' ? process : {
  env: /** @type {Config} */({ NODE_ENV: 'development' }),
  argv: /** @type {string[]} */ ([]),
};

We already should be guarding against undefined it seems.

Edit: NVM I see #269

@tbranyen
Copy link
Owner

I created a reduced test case and can't reproduce this behavior:

tim in ~/tmp/tmp.Fymt36hUrW on from-work-laptop 
λ cat package.json
{
  "dependencies": {
    "diffhtml": "^1.0.0-beta.27",
    "webpack": "^5.72.0",
    "webpack-cli": "^4.9.2"
  }
}
tim in ~/tmp/tmp.Fymt36hUrW on from-work-laptop 
λ cat index.js       
import { innerHTML } from 'diffhtml';

console.log(innerHTML);
tim in ~/tmp/tmp.Fymt36hUrW on from-work-laptop 
λ webpack ./index.js 
asset main.js 24.2 KiB [compared for emit] [minimized] (name: main)
orphan modules 111 KiB [orphan] 34 modules
runtime modules 670 bytes 3 modules
./index.js + 34 modules 111 KiB [built] [code generated]

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value.
Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/

webpack 5.72.0 compiled with 1 warning in 921 ms
tim in ~/tmp/tmp.Fymt36hUrW on from-work-laptop 
λ node ./dist/main.js
[Function: Fe]

@justjake
Copy link
Contributor Author

Wait, how will running an example code in Node (which provides process globally) illustrate that process is (not) required? 😓

@justjake
Copy link
Contributor Author

justjake commented Apr 14, 2022

Here's a repo with the repro steps you tried, plus an HTML page for trying out the bundle Webpack produces:

https://github.com/justjake/diffhtml-repro

Instructions to repro:

  • npm install
  • npm run build
  • npx serve
  • Open the npx serve URL in your browser.

image

@tbranyen
Copy link
Owner

Looks like webpack does weird things when it sees process in modules, even when the reference is pulled from a module export and not the global scope. I renamed all the internal references to nodeProcess instead of process and the error goes away:

image

tbranyen added a commit that referenced this issue Apr 14, 2022
This fixes GH-268. Webpack has issues with the process variable and by
renaming the internal usage we can ensure that it points to our
representation and not the external one.
@tbranyen
Copy link
Owner

Kind of annoying fix, but it is important to support webpack defaults.

https://github.com/tbranyen/diffhtml/pull/270/files

tbranyen added a commit that referenced this issue Apr 14, 2022
This fixes GH-268. Webpack has issues with the process variable and by
renaming the internal usage we can ensure that it points to our
representation and not the external one.
tbranyen added a commit that referenced this issue Apr 14, 2022
* Rename process to internalProcess

This fixes GH-268. Webpack has issues with the process variable and by
renaming the internal usage we can ensure that it points to our
representation and not the external one.

* Ensure rollup respects the symbol change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants