Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

use toml-edit for modifying provided wrangler.toml for wrangler generate to preserve formatting #1745

Merged
merged 1 commit into from
Feb 10, 2021
Merged

use toml-edit for modifying provided wrangler.toml for wrangler generate to preserve formatting #1745

merged 1 commit into from
Feb 10, 2021

Conversation

xortive
Copy link
Contributor

@xortive xortive commented Jan 25, 2021 •
edited
Loading

This came up when testing out #1677, wrangler generate's outputted toml for the testing durable objects wrangler.toml is rather ugly. Keeping a nice, understandable format for wrangler.toml is an important part of reducing user confusion, and I think it's worth the small risk of having two toml parsers in wrangler.

example output from this pr vs current implementation coming:

@xortive xortive requested a review from a team January 25, 2021 17:28
@xortive
Copy link
Contributor Author

xortive commented Jan 25, 2021

what wrangler generate on #1677 outputs for https://github.com/xortive/durable-objects-template

name = "current"
type = "javascript"
account_id = ""
workers_dev = true
route = ""
zone_id = ""

[builder_config]
build_command = "npm install && npm run build"
build_dir = "dist"
upload_format = "modules"
src_dir = "src"
[[durable_objects.uses]]
binding = "Counter"
namespace_name = "Counter"

[[durable_objects.implements]]
namespace_name = "Counter"
class_name = "Counter"

what this PR outputs:

name = "toml-edit"
type = "javascript"
account_id = ""
workers_dev = true
route = ""
zone_id = ""

[builder_config]
build_command = "npm install && npm run build"
upload_format = "modules"
src_dir = "src"
build_dir = "dist"

[durable_objects]
implements = [{class_name = "Counter", namespace_name = "Counter"}]
uses = [{binding = "Counter", namespace_name = "Counter"}]

@xortive
Copy link
Contributor Author

xortive commented Jan 25, 2021

This PR needs to be tested with our various templates as well as workers sites, to ensure we aren't now making a confusing toml.

ags799
ags799 reviewed Jan 26, 2021
View reviewed changes
src/settings/toml/site.rs Show resolved Hide resolved
src/settings/toml/manifest.rs Outdated Show resolved Hide resolved
src/settings/toml/manifest.rs Show resolved Hide resolved
@xortive xortive force-pushed the malonso/preserve-format-generate branch from 1cbd25d to f769449 Compare January 27, 2021 19:07
@xortive
Copy link
Contributor Author

xortive commented Jan 27, 2021

Tested wrangler generate --site

account_id = ""
name = "site-test"
type = "webpack"
workers_dev = true
site = { bucket = "./public" }

Outputs inline table instead of separate table currently. This is equivalent to the existing output, and satisfies a TODO comment in manifest.rs as a better solution to #798

@xortive
Copy link
Contributor Author

xortive commented Jan 27, 2021

took a look at the templates linked to from the docs and none of them are doing anything unique in their wrangler.toml except for the sites one and the scala KV one, and both of those outputted good tomls when testing.

@xortive
Copy link
Contributor Author

xortive commented Jan 28, 2021

rerunning tests since windows test flaked on preview

@xortive xortive changed the base branch from master to malonso/modules-do February 10, 2021 17:33
@xortive xortive merged commit b86946b into malonso/modules-do Feb 10, 2021
@delete-merged-branch delete-merged-branch bot deleted the malonso/preserve-format-generate branch February 10, 2021 17:33
xortive added a commit that referenced this pull request Feb 25, 2021
@xortive
use toml-edit for modifying provided wrangler.toml for wrangler gener…
1cf5fe6 
…ate so format is preserved (#1745)
xortive added a commit that referenced this pull request Feb 25, 2021
@xortive
use toml-edit for modifying provided wrangler.toml for wrangler gener…
9265785 
…ate so format is preserved (#1745)
xortive added a commit that referenced this pull request Mar 4, 2021
@xortive
use toml-edit for modifying provided wrangler.toml for wrangler gener…
74b3f22 
…ate so format is preserved (#1745)
caass pushed a commit that referenced this pull request Mar 15, 2021
@xortive
use toml-edit for modifying provided wrangler.toml for wrangler gener…
dc0dcfc 
…ate so format is preserved (#1745)
xortive added a commit that referenced this pull request Apr 15, 2021
@xortive
Squashed commit of the following:
538242a 
rename build_dir to package_dir

reasoning: package_dir is where package.json is, I'm not sure why to
call it build_dir when it's not the directory where we output build artifacts.
we may want to use build_dir in the future, such as for custom build config

commit e648284
Author: [email protected] <[email protected]>
Date:   Tue Dec 15 10:56:36 2020 -0600

    better build logging

commit a271ffc
Author: [email protected] <[email protected]>
Date:   Mon Dec 14 11:45:41 2020 -0600

    add script-format field, remove Option<T> from builder config

commit 193ba02
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 13:53:30 2020 -0600

    coalesce bundler TargetType under javascript, using config to distinguish

commit db9c356
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 15:09:39 2020 -0600

    rename bundle_config to builder_config, and related names

commit e5780a0
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 13:22:14 2020 -0600

    remove check linter

commit 1c3b588
Merge: 2a3b5b3 cbbed17
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 13:02:33 2020 -0600

    Merge remote-tracking branch 'caass/master' into malonso/modules-do

commit cbbed17
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:47:05 2020 -0500

    Rewrite gzip buffer-building strategy to maybe be friendlier to rayon

commit eeb38cf
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:20:53 2020 -0500

    forgot to run clippy again lol

commit 6cba17b
Merge: b000958 04584e5
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:10:03 2020 -0500

    Merge branch 'cloudflare' into master

commit b000958
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:07:17 2020 -0500

    Apply suggestions from code review

    - Check total bundle size, not individual files
    - Replace hyphens with underscores in wrangler.toml
    - Rename MAX_FILE_SIZE to MAX_BUNDLE_SIZE
    - Duplicate comment about swc_common::SourceMap vs sourcemap::SourceMap
    - rename entry & entry_file to module & module_path

commit 000a79c
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 13:42:40 2020 -0400

    Fix clippy warnings

commit d84f01b
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 13:19:39 2020 -0400

    Add some documentation

commit 90c3489
Merge: 448cf1e 11fe7c1
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 12:45:07 2020 -0400

    Merge branch 'cloudflare' into master

commit 448cf1e
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 12:39:34 2020 -0400

    Rewrite our validation strategy one last time

    Everything compiles now, which is nice. We just need to actually
    implement Lintable for JavaScript and we should have version one ready
    to go.

commit ea1880e
Author: Cassandra Fridkin <[email protected]>
Date:   Sat Oct 17 22:52:11 2020 -0400

    Things might be broken-er in this commit

    But we're going for the restructure. again. round four. this time,
    with macros. because i'm sick of defining Lintable for all of these
    structs

commit 031329c
Author: Cassandra Fridkin <[email protected]>
Date:   Sun Oct 4 00:00:10 2020 -0400

    Add another monstrosity of a switch statement for expressions

    I guess...now I just have to...implement Lintable for all these
    different types of expressions...yippee...

commit b3bfde3
Author: Cassandra Fridkin <[email protected]>
Date:   Sat Oct 3 16:09:59 2020 -0400

    Clean up some TODOs because we did them

commit 0ace2ac
Author: Cassandra Fridkin <[email protected]>
Date:   Sat Oct 3 11:23:45 2020 -0400

    Define Lintable, Parseable, and Validate traits

    I keep making tons of changes without committing but I think that it
    seems bigger than it actually is -- mostly what I've done here is
    define a few traits

    Lintable: a struct is able to lint itself with a given argument to check against
    Parseable: a struct is able to create Self from a given input
    Validate: a struct satisfies both Lintable and Parseable

    The JS boilerplate is mostly done -- since we actually want to lint expressions,
    and not statements, the bulk of the code at the moment is just about
    getting to the expressions in each statement. Essentially a glorifed
    switch case with a little recursion mixed in.

    The webassembly is all todo!(), i just wanted to get typechecking working.

    Basically, this is just yet another commit before we actually parse any
    AST. But I did write some documentation, which is cool.

commit 2ba95fb
Author: Cassandra Fridkin <[email protected]>
Date:   Wed Sep 30 23:13:10 2020 -0400

    Add some tests

    Looks like we will have to wait until the next commit to parse any AST

commit 409063e
Merge: 9da3a2e d6068cc
Author: Cassandra Fridkin <[email protected]>
Date:   Wed Sep 30 20:53:06 2020 -0400

    Merge branch 'cloudflare' into master

commit 9da3a2e
Author: Cassandra Fridkin <[email protected]>
Date:   Wed Sep 30 20:47:08 2020 -0400

    Load the AST & restructure BundlerOutput for the last time

    This restructuring is because I think it makes more sense to
    have each FileType run its own checks. This also cleanly bridges
    between the functions in `js.rs` and `wasm.rs` and the BundlerOutput
    struct in `mod.rs`.

commit e23f88d
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Sep 29 16:36:23 2020 -0400

    Restructure a little bit and mess things up with async

    I think it makes sense to actually create a `Bundler` struct because
    like...eventually in my head it makes sense to have one `Bundler`
    instance that you could like pass around in `wrangler dev` or
    whatever.

    I think most of the codebase is more functional-y but i'm sure someone
    will tell me if this is not the preferred style to do this in.

    How many more commits can I make before I have to actually parse any
    javascript? Let's find out, shall we?

commit 0fec4d4
Author: Cassandra Fridkin <[email protected]>
Date:   Mon Sep 28 00:21:24 2020 -0400

    Add a few tests I suppose, and comment out constants we're not using yet

commit e5b4049
Author: Cassandra Fridkin <[email protected]>
Date:   Sun Sep 27 23:18:51 2020 -0400

    Add "None"s for bundle config so rustc stops complaining

commit 7dafd73
Author: Cassandra Fridkin <[email protected]>
Date:   Sun Sep 27 23:17:06 2020 -0400

    Preliminary "file exists" checks

    Also, use the nice AsRef<Path> idiom that works so well

commit 67bfc70
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Sep 24 20:07:24 2020 -0400

    Add bundle as a target

    I think...at least...that that's what this commit does. Def starting
    to get into "oh god i hope i don't break anything" territory.

commit 5f41321
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Sep 24 15:36:56 2020 -0400

    Add "bundle" type

    Not sure *exactly* what this does but i basically just copied the
    parts i thought were relevant from the site code. We have a src dir,
    which is what needs to be watched, and an output dir, which is what
    needs to be analyzed, and a build command, which is what needs to be
    run when we wanna run the build.

commit efd3a44
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Sep 22 22:10:26 2020 -0400

    trivial commit so i can PR

    literally does nothing except break things :blushing:

cleanup: separate out service-worker form building logic

move service_worker specific metadata to service_worker.rs

support uploading module-based scripts

use toml-edit for modifying provided wrangler.toml for wrangler generate so format is preserved (#1745)

Fix deeply nested module imports from collapsing into the root upload directory

Add path_slash dependency, so paths work on Windows

cleanup: separate out publish message building logic

Add spinners for upload and deploy

Fix Workers Sites initialization entry-point reference

Allow non-Webpack Workers Sites

Throw errors on Rust types and when missing build commands on JS types

Extract out error message

prefix generated module names with ./

Implement issues/1841 (globbing for module types / redo upload_format config)

move ModuleConfig to project_assets.rs, and create it from contents of enum variant

use empty struct variant for ServiceWorker so serde's deny_unknown_fields works

Don't generate a usage_model field by default in wrangler.toml, which breaks sites init
xortive added a commit that referenced this pull request Apr 15, 2021
@xortive
Squashed commit of the following:
d4df7ee 
rename build_dir to package_dir

reasoning: package_dir is where package.json is, I'm not sure why to
call it build_dir when it's not the directory where we output build artifacts.
we may want to use build_dir in the future, such as for custom build config

commit e648284
Author: [email protected] <[email protected]>
Date:   Tue Dec 15 10:56:36 2020 -0600

    better build logging

commit a271ffc
Author: [email protected] <[email protected]>
Date:   Mon Dec 14 11:45:41 2020 -0600

    add script-format field, remove Option<T> from builder config

commit 193ba02
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 13:53:30 2020 -0600

    coalesce bundler TargetType under javascript, using config to distinguish

commit db9c356
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 15:09:39 2020 -0600

    rename bundle_config to builder_config, and related names

commit e5780a0
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 13:22:14 2020 -0600

    remove check linter

commit 1c3b588
Merge: 2a3b5b3 cbbed17
Author: [email protected] <[email protected]>
Date:   Fri Dec 11 13:02:33 2020 -0600

    Merge remote-tracking branch 'caass/master' into malonso/modules-do

commit cbbed17
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:47:05 2020 -0500

    Rewrite gzip buffer-building strategy to maybe be friendlier to rayon

commit eeb38cf
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:20:53 2020 -0500

    forgot to run clippy again lol

commit 6cba17b
Merge: b000958 04584e5
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:10:03 2020 -0500

    Merge branch 'cloudflare' into master

commit b000958
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Nov 3 00:07:17 2020 -0500

    Apply suggestions from code review

    - Check total bundle size, not individual files
    - Replace hyphens with underscores in wrangler.toml
    - Rename MAX_FILE_SIZE to MAX_BUNDLE_SIZE
    - Duplicate comment about swc_common::SourceMap vs sourcemap::SourceMap
    - rename entry & entry_file to module & module_path

commit 000a79c
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 13:42:40 2020 -0400

    Fix clippy warnings

commit d84f01b
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 13:19:39 2020 -0400

    Add some documentation

commit 90c3489
Merge: 448cf1e 11fe7c1
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 12:45:07 2020 -0400

    Merge branch 'cloudflare' into master

commit 448cf1e
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Oct 29 12:39:34 2020 -0400

    Rewrite our validation strategy one last time

    Everything compiles now, which is nice. We just need to actually
    implement Lintable for JavaScript and we should have version one ready
    to go.

commit ea1880e
Author: Cassandra Fridkin <[email protected]>
Date:   Sat Oct 17 22:52:11 2020 -0400

    Things might be broken-er in this commit

    But we're going for the restructure. again. round four. this time,
    with macros. because i'm sick of defining Lintable for all of these
    structs

commit 031329c
Author: Cassandra Fridkin <[email protected]>
Date:   Sun Oct 4 00:00:10 2020 -0400

    Add another monstrosity of a switch statement for expressions

    I guess...now I just have to...implement Lintable for all these
    different types of expressions...yippee...

commit b3bfde3
Author: Cassandra Fridkin <[email protected]>
Date:   Sat Oct 3 16:09:59 2020 -0400

    Clean up some TODOs because we did them

commit 0ace2ac
Author: Cassandra Fridkin <[email protected]>
Date:   Sat Oct 3 11:23:45 2020 -0400

    Define Lintable, Parseable, and Validate traits

    I keep making tons of changes without committing but I think that it
    seems bigger than it actually is -- mostly what I've done here is
    define a few traits

    Lintable: a struct is able to lint itself with a given argument to check against
    Parseable: a struct is able to create Self from a given input
    Validate: a struct satisfies both Lintable and Parseable

    The JS boilerplate is mostly done -- since we actually want to lint expressions,
    and not statements, the bulk of the code at the moment is just about
    getting to the expressions in each statement. Essentially a glorifed
    switch case with a little recursion mixed in.

    The webassembly is all todo!(), i just wanted to get typechecking working.

    Basically, this is just yet another commit before we actually parse any
    AST. But I did write some documentation, which is cool.

commit 2ba95fb
Author: Cassandra Fridkin <[email protected]>
Date:   Wed Sep 30 23:13:10 2020 -0400

    Add some tests

    Looks like we will have to wait until the next commit to parse any AST

commit 409063e
Merge: 9da3a2e d6068cc
Author: Cassandra Fridkin <[email protected]>
Date:   Wed Sep 30 20:53:06 2020 -0400

    Merge branch 'cloudflare' into master

commit 9da3a2e
Author: Cassandra Fridkin <[email protected]>
Date:   Wed Sep 30 20:47:08 2020 -0400

    Load the AST & restructure BundlerOutput for the last time

    This restructuring is because I think it makes more sense to
    have each FileType run its own checks. This also cleanly bridges
    between the functions in `js.rs` and `wasm.rs` and the BundlerOutput
    struct in `mod.rs`.

commit e23f88d
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Sep 29 16:36:23 2020 -0400

    Restructure a little bit and mess things up with async

    I think it makes sense to actually create a `Bundler` struct because
    like...eventually in my head it makes sense to have one `Bundler`
    instance that you could like pass around in `wrangler dev` or
    whatever.

    I think most of the codebase is more functional-y but i'm sure someone
    will tell me if this is not the preferred style to do this in.

    How many more commits can I make before I have to actually parse any
    javascript? Let's find out, shall we?

commit 0fec4d4
Author: Cassandra Fridkin <[email protected]>
Date:   Mon Sep 28 00:21:24 2020 -0400

    Add a few tests I suppose, and comment out constants we're not using yet

commit e5b4049
Author: Cassandra Fridkin <[email protected]>
Date:   Sun Sep 27 23:18:51 2020 -0400

    Add "None"s for bundle config so rustc stops complaining

commit 7dafd73
Author: Cassandra Fridkin <[email protected]>
Date:   Sun Sep 27 23:17:06 2020 -0400

    Preliminary "file exists" checks

    Also, use the nice AsRef<Path> idiom that works so well

commit 67bfc70
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Sep 24 20:07:24 2020 -0400

    Add bundle as a target

    I think...at least...that that's what this commit does. Def starting
    to get into "oh god i hope i don't break anything" territory.

commit 5f41321
Author: Cassandra Fridkin <[email protected]>
Date:   Thu Sep 24 15:36:56 2020 -0400

    Add "bundle" type

    Not sure *exactly* what this does but i basically just copied the
    parts i thought were relevant from the site code. We have a src dir,
    which is what needs to be watched, and an output dir, which is what
    needs to be analyzed, and a build command, which is what needs to be
    run when we wanna run the build.

commit efd3a44
Author: Cassandra Fridkin <[email protected]>
Date:   Tue Sep 22 22:10:26 2020 -0400

    trivial commit so i can PR

    literally does nothing except break things :blushing:

cleanup: separate out service-worker form building logic

move service_worker specific metadata to service_worker.rs

support uploading module-based scripts

use toml-edit for modifying provided wrangler.toml for wrangler generate so format is preserved (#1745)

Fix deeply nested module imports from collapsing into the root upload directory

Add path_slash dependency, so paths work on Windows

cleanup: separate out publish message building logic

Add spinners for upload and deploy

Fix Workers Sites initialization entry-point reference

Allow non-Webpack Workers Sites

Throw errors on Rust types and when missing build commands on JS types

Extract out error message

prefix generated module names with ./

Implement issues/1841 (globbing for module types / redo upload_format config)

move ModuleConfig to project_assets.rs, and create it from contents of enum variant

use empty struct variant for ServiceWorker so serde's deny_unknown_fields works
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nataliescottdavidson nataliescottdavidson nataliescottdavidson approved these changes

@ags799 ags799 ags799 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xortive @ags799 @nataliescottdavidson