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

nnbd support #92

Open
alextekartik opened this issue Feb 19, 2021 · 12 comments
Open

nnbd support #92

alextekartik opened this issue Feb 19, 2021 · 12 comments

Comments

@alextekartik
Copy link
Contributor

Is there a plan to have support for nnbd for both the compilers and packages? Thanks!

@stof
Copy link

stof commented Mar 17, 2021

Part of the work has been done in #90

@Norbert515
Copy link

What's the current status of this? Is there a way to use this with a dart 2.12 package?

@alextekartik
Copy link
Contributor Author

Unfortunately that does not seem to be the case, I manage to use with dart 2.12 only by forcing the usage of non-nnbd packages (for example path: <1.8.0-0) and with the following option in build.yaml:

    dart2js_args:
      # Temp nnbd
      - --no-sound-null-safety

which means using packages that are not maintained anymore.

I still believe that this is a viable server side solution (and currently the only way to use dart in firebase functions and triggers).

@lexaknyazev
Copy link
Contributor

In the current state, build_node_compilers cannot correctly handle projects with mixed sound / unsound null-safety, probably due to use of older APIs.

I tried aligning it with the current stable build_web_compilers but quickly realized that the only practical difference for dart2js compiler is prepending node_preamble to the compiled output. By switching to build_web_compilers and prepending the preamble myself via a grinder task, I was able to remove the dependency on build_node_compilers altogether.

Given how quickly build_* packages evolve, I think there's a strong case for upstreaming that single difference and exposing it directly from build_web_compilers via an optional flag. Such change would greatly reduce maintenance burden for the node-interop project and improve the health of the ecosystem.

The story is slightly more complicated for DDC because build_node_compilers uses modified JS loaders.

/cc @pulyaevskiy @jakemac53

@alextekartik
Copy link
Contributor Author

alextekartik commented Apr 14, 2021

@lexaknyazev Thanks for the info, I was indeed able to compile nnbd packages using your trick. I personally don't really care about ddc as I enforce the usage of dart2js and just build for running (at the end it has to run in firebase functions so I'm not up to here yet). I'm not sure though whether that will remain a long term solution (the dart team does not seem to have the bandwith/willingness to support node).

My build.yaml if it can help others:

targets:
  $default:
    sources:
      - "$package$"
      - "node/**"
      - "lib/**"
    builders:
      build_web_compilers|entrypoint:
        generate_for:
        - node/**
        options:
          # enforce dart2js compiler
          compiler: dart2js

And I can build using the following command:

pub run build_runner build --output=build/

I also have a small script that insert node preamble min js in the generated js file after building.

@lexaknyazev
Copy link
Contributor

node/** is included in the default set of build_runner_core sources, so the sources config section could be omitted in most cases.

@SupposedlySam
Copy link

@lexaknyazev Thanks for the info, I was indeed able to compile nnbd packages using your trick. I personally don't really care about ddc as I enforce the usage of dart2js and just build for running (at the end it has to run in firebase functions so I'm not up to here yet). I'm not sure though whether that will remain a long term solution (the dart team does not seem to have the bandwith/willingness to support node).

My build.yaml if it can help others:

targets:
  $default:
    sources:
      - "$package$"
      - "node/**"
      - "lib/**"
    builders:
      build_web_compilers|entrypoint:
        generate_for:
        - node/**
        options:
          # enforce dart2js compiler
          compiler: dart2js

And I can build using the following command:

pub run build_runner build --output=build/

I also have a small script that insert node preamble min js in the generated js file after building.

@alextekartik, if this solution is still working for you, is there any way you can include the script you created with details on how you run it?

@alextekartik
Copy link
Contributor Author

@SupposedlySam I have an helper package that does the build and provides some common method for building here (build + adding preamble): https://github.com/tekartik/build_node.dart/tree/main/packages/build_node and a raw node example that uses this builder here: https://github.com/tekartik/build_node.dart/tree/main/packages/example/simple_example

@SupposedlySam
Copy link

Thank you @alextekartik! It looks like the nodeBuild function from your helper package does all the heavy lifting of adding the preamble to all dart files. I'll give this a try!

@SupposedlySam
Copy link

After updating my pubspec to use build_web_compilers: ^3.0.0 and using the build.yaml example you provided above, it looks like there may be some kind of configuration I'm still missing. Any direction with this error?

Skipping compiling functions|node/index.dart with dart2js because some of its
transitive libraries have sdk dependencies that are not supported on this platform:

node_io|lib/src/file.dart
node_io|lib/src/stdout.dart
node_io|lib/node_io.dart
node_io|lib/src/streams.dart
node_io|lib/src/link.dart
node_io|lib/src/http_server.dart
node_io|lib/src/directory.dart
node_io|lib/src/file_system.dart
node_io|lib/src/http_headers.dart
node_io|lib/src/network_interface.dart
node_io|lib/src/internet_address.dart
node_io|lib/src/file_system_entity.dart
file|lib/src/io.dart

@alextekartik
Copy link
Contributor Author

@SupposedlySam Are you using node_io: ^2.1.0 ?

dart pub upgrade
dart pub outdated --mode=null-safety

should help you find what is wrong

@dannnnthemannnn
Copy link

@SupposedlySam I updated this flag to be false and it seems to have worked (after a few more tweaks) https://github.com/dart-lang/build/blob/3620bd127b74c305a53989595175b6e6f83ea9e0/build_web_compilers/lib/src/dart2js_bootstrap.dart#L54

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

No branches or pull requests

6 participants