-
Notifications
You must be signed in to change notification settings - Fork 700
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
Update JS compiler content for v2.18 #4193
Conversation
dc48ceb
to
3f88f00
Compare
src/tools/dart-compile.md
Outdated
Generated: /tmp/myapp | ||
``` | ||
|
||
To run your compiled app, change to the `/tmp` directory and run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be simpler if it was just /tmp/myapp
without having to change directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with mit.
src/tools/dartdevc.md
Outdated
Dart 2.18 removes the `dartdevc` command-line tool from the Dart | ||
package, but retains the dartdevc compiler. | ||
|
||
Use [`dart compile js`](/tools/dart-compile#js) to compile Dart code |
There was a problem hiding this comment.
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 is true. compile js
will produce non-debuggable JS. I think maybe we need to point to webdev instead, @sigmundch ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually looks like the removed content below might all still work, and just need to be retained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all falls under the dartdevc
command. I can excise some parts and put them elsewhere, like webdev
. I thought you were looking for removing anything other than dart compile js
, so I tried to stay in that lane. Can you indicate which lines I should move? All of them, changing dartdevc
to webdev serve
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My recommendation here is that we are not downplaying one command over others. Both commands are serving different purposes and each is valid for the journey the developer is at the moment (I provided additional details in the planning doc if that helps).
That means that we'd keep both references to web development and web production compilers, but change the sample commands. If a command used dart2js
, we turn it into dart compile js
, if a command used dartdevc
, we turn it into webdev serve
.
In the past, calling dartdevc
directly was not recommended anyways, and not something I expect we had said to do in the documentation (we used the term to refer to the development compiler, but I'd be surprised if we had examples to show people how to call it directly). So I expect for the most part we used webdev serve
to describe those use cases, and we can simply preserve good chunks of the old documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct - in this case we'd point them at webdev serve
There is still a page for webdev below, so rather than retaining the content of the dartdevc.md file, I'd recommend selecting the relevant pieces to preserve that make sense to show in the context of webdev. From looking at the contents on this file, thought, I don't think there is much I'd keep (all content about module format, etc, are not really that relevant to our developers).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a requirement that we keep old pages around? or is it OK to delete them altogether?
After looking at this more closely, I'm no longer sure I see the value in keeping around both the dartdevc.md and the dart2js.md pages. So I'd be inclined in deleting them both. If we have to keep them, then I would suggest to trim them down further. For example, by deleting anything that is describing the compilers beyond production/development (e.g. I'd suggest deletion the whole explanation of modules and AMD).
One suggestion:
- "Version note: Since Dart 2.18 the
dart2js
web production compiler is only available fromdart compile js
(link)", and - "Version note: Since Dart 2.18 the
dartdevc
web development compiler is only available fromwebdev serve
(link)."
Another suggestion (I slightly prefer this one):
- "dart2js is Dart's web production compiler. Version note: Since Dart 2.18 the
dart2js
compiler can only be invoked fromdart compile js
. It is also used internally by deployment tools likewebdev build
", and - "dartdevc is Dart's web development compiler. Version note: Since Dart 2.18 the
dartdevc
compiler cannot be invoked directly, but it is used internally by development tools likewebdev serve
."
**Only dart2js supports deferred loading.** | ||
Flutter, the Dart VM, and dartdevc don't support deferred loading. | ||
For more information, see | ||
**Only `dart compile js` supports deferred loading.** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI - I believe some support for deferred loading has been added to the VM and we are working towards adding it to the dartdevc workflow (webdev). That's tangential to this change, so I wouldn't change anything here, just wanted to give you a heads up :)
src/_guides/whats-new.md
Outdated
|
||
In addition to bug fixes and incremental improvements, | ||
we made the following changes to this site: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is your intent to also add here some bullet points about these recent changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this was accidentally included as that is being handled in #4195
src/faq.md
Outdated
The _development_ compiler ([dartdevc][]) supports only Chrome. | ||
You _might_ be able to use other modern browsers | ||
The _development_ web compiler supports Chrome only. | ||
You _might_ be able to use other modern browsers based on Chromium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend reverting this line change - only Edge is based on chromium, FF and Safari are not.
src/faq.md
Outdated
The _development_ compiler ([dartdevc][]) supports only Chrome. | ||
You _might_ be able to use other modern browsers | ||
The _development_ web compiler supports Chrome only. | ||
You _might_ be able to use other modern browsers based on Chromium | ||
(Chromium Edge, Firefox, and Safari). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider removing there "Chromium", since now the default is that Edge is a chromium based browser?
src/tools/dartdevc.md
Outdated
Dart 2.18 removes the `dartdevc` command-line tool from the Dart | ||
package, but retains the dartdevc compiler. | ||
|
||
Use [`dart compile js`](/tools/dart-compile#js) to compile Dart code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My recommendation here is that we are not downplaying one command over others. Both commands are serving different purposes and each is valid for the journey the developer is at the moment (I provided additional details in the planning doc if that helps).
That means that we'd keep both references to web development and web production compilers, but change the sample commands. If a command used dart2js
, we turn it into dart compile js
, if a command used dartdevc
, we turn it into webdev serve
.
In the past, calling dartdevc
directly was not recommended anyways, and not something I expect we had said to do in the documentation (we used the term to refer to the development compiler, but I'd be surprised if we had examples to show people how to call it directly). So I expect for the most part we used webdev serve
to describe those use cases, and we can simply preserve good chunks of the old documentation.
@@ -373,34 +370,18 @@ For example, the `dart:io` library | |||
provides access to operating system files and directories with APIs not | |||
available to the browser. | |||
|
|||
### Q. Why does Dart have two JavaScript compilers, dartdevc and dart2js? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still preserve this Q. Maybe instead of "why two compilers", ask "why does Dart have two ways to compile to JavaScript"
In the paragraph below we can however remove the references to the actual compiler names. For example:
Each developer journey has different requirements, so Dart generates JavaScript differently
depending on what you need to do. You don't usually have to worry
about which compiler you're using, because the [webdev][] tool
chooses the right compiler for your use case. When you're developing your app,
webdev chooses a development web compiler, which is modular and supports
incremental compilation so you can quickly see the results of your edits.
When you're building your app for deployment, webdev chooses the web
production compiler (same asdart compile js
) which uses techniques such
as tree shaking to produce optimized code.
|
||
We don't claim that all Dart code will run faster | ||
than handwritten JavaScript, when compiled to JavaScript, | ||
but we're working to make the common cases fast. | ||
|
||
### Q. How can I write Dart code that compiles to performant JavaScript? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an important reference, so I would prefer not to remove it.
src/null-safety/faq.md
Outdated
@@ -173,7 +173,7 @@ The assert will be unnecessary when everything is fully migrated, but for now it | |||
## How should I migrate a runtime null check that now shows as unnecessary? | |||
|
|||
An explicit runtime null check, for example `if (arg == null) throw | |||
ArgumentError(...)`, will be flagged as an unnecessary comparison if you make | |||
ArgumentError(...)`, will be optionged as an unnecessary comparison if you make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: undo edit?
src/null-safety/faq.md
Outdated
@@ -319,72 +319,75 @@ and situations where a null value is really expected. So the tool tells you what | |||
it knows ("it looks like this condition will always be false!") and lets you | |||
decide what to do. | |||
|
|||
## What should I know about dart2js and null safety? | |||
## What should I know about compiling and null safety? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compiling
here is too general (since that includes compilation we do for AOT, and other non-web work).
I'd either use dart compile js
or "web production compiles" here.
src/null-safety/faq.md
Outdated
For a long time, the dart2js compiler has had optimizations specifically | ||
targeting null values and null checks. Because of that, null safety is not | ||
expected to affect much the output of the compiler. | ||
The compiler optimizes null values and null checks. So null safety |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is some missing pieces in this paragraph that it's worth adding. I realize now that when I first wrote this I didn't word this properly to really convey what I was trying to say, so while your edits make sense looking at this with fresh eyes, I feel that some of the nuance that I was trying to convey here gets lost in the process.
To provide additional context. Developers know that null safety bring many benefits. One such benefit is that it will reduce code-size and improve performance of apps compiled natively (e.g. flutter and AOT targets). Many developers believe that the same benefits will come to JavaScript apps, but in reality the benefits will be much smaller. This Q/A was added because we wanted to set expectations appropriately.
My goal with the first sentence "For a long time ..." was to provide the reason behind this. That is, while our compiler learns from null-safety and it makes the compiler job easier, it's just that in the past there were many optimizations added to the production web compiler to specifically recover the cost of null checks, and as a result the benefits of null safety are not as easily observable when looking at the output of the compiler.
There is probably a better way to convey all of this :). I know this is tangential to your edits, so I'd be fine keeping the original wording for now.
src/overview.md
Outdated
a production time compiler (dart2js). | ||
Both compilers translate Dart into JavaScript. | ||
Dart can compile for debugging or production. | ||
Its web compiler translates Dart into JavaScript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically there are 2 ocmpilers though... another thought here is to keep the original wording, but remove the words in parenthesis from the original sentence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a different update. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "Its web compilers translate Dart into JavaScript" (plural compilers vs singluar compiler?)
Alternatively, combining both sentences:
Web platform: For apps targeting the web, Dart can compile to JavaScript, both for development and production purposes.
src/tools/dart-compile.md
Outdated
The [`webdev build`][] command, by default, | ||
also produces minified, deployable JavaScript. | ||
The [`webdev serve`][] command, by default, | ||
produces JavaScript modules to compile web apps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: delete "to compile web apps" ?
are safe for all programs. | ||
{{site.alert.note}} | ||
With `-O2`, string representations of types are no longer the same as | ||
those in the Dart VM and when compiled with [dartdevc][]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: one more reference to dartdevc to remove here.
src/tools/dartdevc.md
Outdated
Dart 2.18 removes the `dartdevc` command-line tool from the Dart | ||
package, but retains the dartdevc compiler. | ||
|
||
Use [`dart compile js`](/tools/dart-compile#js) to compile Dart code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct - in this case we'd point them at webdev serve
There is still a page for webdev below, so rather than retaining the content of the dartdevc.md file, I'd recommend selecting the relevant pieces to preserve that make sense to show in the context of webdev. From looking at the contents on this file, thought, I don't think there is much I'd keep (all content about module format, etc, are not really that relevant to our developers).
src/web/deployment.md
Outdated
as described in the [build_web_compilers page.][build_web_compilers] | ||
[Use the `webdev build` command][build] to create a deployable version | ||
of your app. This command converts your code to JavaScript and saves | ||
the result as `build/web/main.dart.js`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with most of the deletions here, but it may be worth including also a link to the dart2js options (I wouldn't document them individually here, but refer to the fact that under the hood webdev build
invokes dart compile js
, so all options available there can also be used to configure a webdev build
.
src/_guides/whats-new.md
Outdated
## August 30, 2022: 2.18 release | ||
|
||
This section lists notable changes made from May 12, 2022, | ||
through August 30, 2022. | ||
For details about the 2.18 release, | ||
see [Dart 2.18: ][], and the [change log][https://github.com/dart-lang/sdk/blob/main/CHANGELOG.md#2180]. | ||
|
||
In addition to bug fixes and incremental improvements, | ||
we made the following changes to this site: | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## August 30, 2022: 2.18 release | |
This section lists notable changes made from May 12, 2022, | |
through August 30, 2022. | |
For details about the 2.18 release, | |
see [Dart 2.18: ][], and the [change log][https://github.com/dart-lang/sdk/blob/main/CHANGELOG.md#2180]. | |
In addition to bug fixes and incremental improvements, | |
we made the following changes to this site: |
@@ -0,0 +1,20 @@ | |||
No cross-compilation support ([issue 28617][]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you commit this by accident? It seems unrelated to this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I yanked out this block from the dart compile
file to cover exe
and aot-snapshot
more cleanly. It is unrelated. Apologies for the extra change.
The mapping is trivial: `Float32List` becomes a `Float32Array`. | ||
One exception exists: the production web compiler does not support | ||
64-bit integers: `Int64List` or `Uint64List`. Compiling Dart code with | ||
either of those lists results in a runtime exception. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI, semantic line breaks like these are a convention the Dart and Flutter sites often use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just learned what this is. Great point. I'll fix some here and others later.
|
||
You must include this check if the program is a mixed-version one. | ||
Until everything is fully migrated and the code switches to running | ||
with sound null safety, `arg` might be set to `null`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated to js compilation, not sure if this was committed by accident
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to clean up the language and format around this check. It's the same content as what was there originally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the changes look great, thanks @atsansone!
I defer to the proper editors here to give the final stamp of approval 😊
: The Dart dev compiler, a modular Dart-to-JavaScript compiler. | ||
IDEs and the `webdev` CLI use dartdevc when running a development server. | ||
|
||
: A CLI to build and serve Dart web apps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider adding back a bullet point equivalent to dart2js
(maybe it would be dart compile js
instead?).
It seems these were the primary links we had to steer readers towards the more detailed articles about how to develop and deploy apps to JavaScript, so without those, this index page feels incomplete.
Also worth nothing something for a follow up (clearly out of scope for this change): it's strange that have a couple workflow articles like web/deployment and web/debugging, but no article on web/development. It would be be consistent and have an article per user journey. My guess the reason is that it overlaps entirely with the content of the webdev.md article, so it would require a bit of editing to figure out the right organization of that content.
@atsansone is this ready for a last review? If so, could you please stage it somewhere? I'm having a bit of trouble visualizing how the pages end up linking together from just the source. |
@mit-mit : The links to the staged pages are in the description at the top of this PR. |
aa27b1a
to
30b6d15
Compare
[Dart Package Configuration File.][] | ||
|
||
`-D<flag>=<value>` | ||
: Defines an environment variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to have a doc on this that can be linked to.
I'm annoyed this page https://api.dart.dev/stable/2.18.0/dart-core/int/int.fromEnvironment.html – and others don't link to something like that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made corrections per @johnpryan .
@@ -0,0 +1,20 @@ | |||
No cross-compilation support ([issue 28617][]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I yanked out this block from the dart compile
file to cover exe
and aot-snapshot
more cleanly. It is unrelated. Apologies for the extra change.
The mapping is trivial: `Float32List` becomes a `Float32Array`. | ||
One exception exists: the production web compiler does not support | ||
64-bit integers: `Int64List` or `Uint64List`. Compiling Dart code with | ||
either of those lists results in a runtime exception. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just learned what this is. Great point. I'll fix some here and others later.
|
||
You must include this check if the program is a mixed-version one. | ||
Until everything is fully migrated and the code switches to running | ||
with sound null safety, `arg` might be set to `null`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to clean up the language and format around this check. It's the same content as what was there originally.
: The Dart dev compiler, a modular Dart-to-JavaScript compiler. | ||
IDEs and the `webdev` CLI use dartdevc when running a development server. | ||
|
||
: A CLI to build and serve Dart web apps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
: A CLI to build and serve Dart web apps. | |
: A CLI to [build](/tools/webdev#build) and [serve](/tools/webdev#serve) Dart JavaScript apps. |
@@ -2470,9 +2470,9 @@ When exactly do assertions work? | |||
That depends on the tools and framework you're using: | |||
|
|||
* Flutter enables assertions in [debug mode.][Flutter debug mode] | |||
* Development-only tools such as [dartdevc][] | |||
* Development-only tools such as [`webdev serve`][] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one more comment (left above)
628d5c2
to
4c509ef
Compare
4c509ef
to
9fdb07d
Compare
@@ -0,0 +1,113 @@ | |||
#### Options {#prod-compile-options} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
includes are typically not used for sections like this. This can make things harder to maintain (the heading type might change depending on the context where it's included.) IMO it's better to just inline things and use includes for banners and other smaller things.
9fdb07d
to
a829540
Compare
Updated two other files Fixed dartdevc conflict Updated per @mit-mit and @sigmundch reviews replaced 'web compiler' with 'JavaScript' compiler Apply suggestions from @johnpryan code review Co-authored-by: John Ryan <ryjohn@google.com> Updated per @kevmoo review Updated missing links per @mit-mit Updated Dockerfile
a829540
to
a52123b
Compare
* 2.18 section in evolution.md, added more to what's new (incomplete) * What's new: added 2.18 section with link to change log. Language evolution: added 2.18 section moving commits to new branch * What's new: added 2.18 section with link to change log. Language evolution: added 2.18 section * whats-new.md added 2.18 changes in docs (and to-be-changed pages), finalized evolution.md summary for 2.18 language changes * changed inference link in evolution.md to point to the Type argument inference section specifically on the Type inference page * adjusted .packages workaround pointer * adjusted to reflect #4193 * remove mention of async in what's new, no changes * incorporate style reviews * Apply suggestions from code review Co-authored-by: Parker Lougheed <parlough@gmail.com> * added more new content * semantic line breaks * review * review * change wording Co-authored-by: Parker Lougheed <parlough@gmail.com>
Removed mentions of
dart2js
anddartdevc
dart compile
dart
Added production debugging section from
dart2js
Removed all content
dart2js
dartdevc