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

fix nodejs environment usage in browser environments for v4.8.5 #253

Merged
merged 3 commits into from
Aug 6, 2023

Conversation

olafbuitelaar
Copy link
Contributor

@olafbuitelaar olafbuitelaar commented Aug 6, 2023

as the NodeJS environment is imported by default, and tries to reference process and hostname(), this fails in a browser. Also it seems to default to always construct with the NodeJS Environment instances. note this pr is against the development branch.
NOTE, if using webpack this release requires disabling;

 resolve: {
               fallback: { 
                "os": false, 
                "util": false,
                "path": false,
            }
        },

@terehov terehov merged commit c4dc319 into fullstack-build:development Aug 6, 2023
@terehov
Copy link
Contributor

terehov commented Aug 6, 2023

Question, why AND instead of OR?

super(isBrowser && typeof(process) === 'undefined' ? BrowserRuntime : NodeRuntime, settings, logObj, isSafari ? 4 : 5);

I think it should be like this:

super(isBrowser || process == undefined ? BrowserRuntime : NodeRuntime, settings, logObj, isSafari ? 4 : 5);

@terehov
Copy link
Contributor

terehov commented Aug 6, 2023

Check out V4.8.6

@olafbuitelaar
Copy link
Contributor Author

good question, i suppose both could be right. the AND, just assumes the window object exists, and the process object certainly doesn't, which in a sense could be more specific.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants