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 isNodeJs and process getters to cli_pkg/js.dart #143

Merged
merged 11 commits into from
Oct 5, 2023
12 changes: 10 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ jobs:
strategy:
matrix:
os: [ubuntu, macos, windows]
dart-channel: [stable, dev]
dart-channel: [stable]
# TODO(nweiz): Re-enable this when
# https://github.com/dart-lang/sdk/issues/52121#issuecomment-1728534228
# is addressed.
# dart-channel: [stable, dev]
shard: [0, 1, 2]
fail-fast: false

Expand Down Expand Up @@ -49,7 +53,11 @@ jobs:

strategy:
matrix:
dart-channel: [stable, dev]
dart-channel: [stable]
# TODO(nweiz): Re-enable this when
# https://github.com/dart-lang/sdk/issues/52121#issuecomment-1728534228
# is addressed.
# dart-channel: [stable, dev]
fail-fast: false

runs-on: ubuntu-latest
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.6.0

* Add `isNodeJs` and `process` getters to `package:cli_pkg/js.dart`.

## 2.5.0

* Add a `wrapJSException()` function in the new `package:cli_pkg/js.dart`
Expand Down
78 changes: 78 additions & 0 deletions browser_library_test/test/node_js_test.dart
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this should be in browser_library_test, since it's not running in a browser and it's not loading a JS library exposed by the package. It would probably make more sense to put it in test/node_js_test.dart.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is actually running in a browser 🤔, just without any other libraries involved. There are tests for running within Node already. I will rename this file to make it less confusing.

github results showing "dart test -p chrome" list of executed tests which includes this file on testing

I guess it's a good idea to expand browser_test_shared.dart to verify none of those libraries would incorrectly return isNodeJs == true

Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import 'dart:js_interop';
import 'dart:js_util';

import 'package:cli_pkg/js.dart';
import 'package:test/test.dart';

@TestOn('browser')
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
void main() {
tearDown(() => delete<Object>(globalThis, 'process'));

const nonNodeJsProcessTestCases = <String, Map<String, Map<String, String>>>{
'an empty process': {},
'a process with empty release': {'release': {}},
'a process with non-Node.JS release name': {
'release': {'name': 'hello'}
},
};

const fakeNodeJsProcess = {
'release': {'name': 'node'}
};

group('isNodeJs', () {
test("returns 'false' from the browser", () {
expect(isNodeJs, isFalse);
});

for (final entry in nonNodeJsProcessTestCases.entries) {
final caseName = entry.key;
final processJson = entry.value.jsify();

test("returns 'false' when $caseName exists in the 'window'", () {
setProperty(globalThis, 'process', processJson);
expect(isNodeJs, isFalse);
});
}

test("returns 'true' with a fake Node.JS process", () {
setProperty(globalThis, 'process', fakeNodeJsProcess.jsify());
expect(isNodeJs, isTrue);
});
});

group('process', () {
test("returns 'null' from the browser", () {
expect(process, isNull);
});

for (final entry in nonNodeJsProcessTestCases.entries) {
final caseName = entry.key;
final processJson = entry.value.jsify();

test("returns 'null' when $caseName exists in the 'window'", () {
setProperty(globalThis, 'process', processJson);
expect(process, isNull);
});
}

test("returns a fake process if it fakes being a Node.JS environment", () {
setProperty(globalThis, 'process', fakeNodeJsProcess.jsify());
expect(process.jsify().dartify(), fakeNodeJsProcess);
});
});
}
32 changes: 32 additions & 0 deletions lib/js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,39 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import 'package:js/js.dart';
import 'package:js/js_util.dart';
import 'package:node_interop/process.dart';

@JS('process')
external final Process? _process; // process is null in the browser

/// This extension adds `maybe<Property>` getters that return non-nullable
/// properties with a nullable type.
extension PartialProcess on Process {
/// Returns [release] as nullable.
Release? get maybeRelease => release;
}

/// Whether the script is being executed in a Node.JS environment.
bool get isNodeJs => _process?.maybeRelease?.name == 'node';

/// Returns the NodeJs [Process] value only when the script is running in
/// Node.JS, otherwise returns `null`.
///
/// By checking whether the script is running in Node.JS we can avoid returning
/// a non-[Process] object if the script is running in a browser, and there is a
/// different `process` object in the `window`. This can happen when a library
/// or framework adds a global variable named `process`. For example, Next.JS
/// adds [`process.env`] to access environment variables on the browser.
///
/// [`process.env`]: https://nextjs.org/docs/pages/building-your-application/configuring/environment-variables#bundling-environment-variables-for-the-browser
///
/// The getter can still return a non-[Process] object if the script is running
/// in a browser and there is a `process` object in the `window` with the value
/// `{release: {name: 'node'}}`. In this case the script must trust the browser
/// is emulating a Node.JS environment.
Process? get process => isNodeJs ? _process : null;
Goodwine marked this conversation as resolved.
Show resolved Hide resolved

/// Whether this Dart code is running in a strict mode context.
///
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cli_pkg
version: 2.5.0
version: 2.6.0
description: Grinder tasks for releasing Dart CLI packages.
homepage: https://github.com/google/dart_cli_pkg

Expand Down
44 changes: 44 additions & 0 deletions test/npm_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,50 @@ void main() {
test("handles a thrown undefined",
() => assertCatchesGracefully('BigInt(undefined)'));
});

test("isNodeJs returns `true` when running in Node.JS", () async {
await d.package(pubspec, _enableNpm, [
_packageJson,
d.dir("bin", [
d.file("foo.dart", """
import 'package:cli_pkg/js.dart';

void main() {
print(isNodeJs);
}
""")
]),
]).create();

await (await grind(["pkg-npm-dev"])).shouldExit();

var process = await TestProcess.start(
"node$dotExe", [d.path("my_app/build/npm/foo.js")]);
expect(process.stdout, emitsInOrder(["true", emitsDone]));
expect(process.shouldExit(0), completes);
});

test("process returns a Process when running in Node.JS", () async {
await d.package(pubspec, _enableNpm, [
_packageJson,
d.dir("bin", [
d.file("foo.dart", """
import 'package:cli_pkg/js.dart';

void main() {
print(process?.release.name);
}
""")
]),
]).create();

await (await grind(["pkg-npm-dev"])).shouldExit();

var process = await TestProcess.start(
"node$dotExe", [d.path("my_app/build/npm/foo.js")]);
expect(process.stdout, emitsInOrder(["node", emitsDone]));
expect(process.shouldExit(0), completes);
});
});
}

Expand Down
Loading