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

Change TOML parser to an active project that supports the latest TOML syntax #1493

Closed
jacob-8 opened this issue May 19, 2021 · 3 comments · Fixed by #1509
Closed

Change TOML parser to an active project that supports the latest TOML syntax #1493

jacob-8 opened this issue May 19, 2021 · 3 comments · Fixed by #1509

Comments

@jacob-8
Copy link
Contributor

jacob-8 commented May 19, 2021

Describe the bug
I upgraded to the most recent wrangler version and expected adding upload.format = "service-worker" to the wrangler.toml (for adapter-cloudflare-workers) file to just work as described in https://developers.cloudflare.com/workers/cli-wrangler/configuration#service-workers but when the adapter (https://github.com/sveltejs/kit/blob/master/packages/adapter-cloudflare-workers/index.js#L61) parses it with the toml package it throws:
Error parsing wrangler.toml: Expected "=", [ \t] or [A-Za-z0-9_\-] but "." found.

To Reproduce

  • npm init svelte@next and npm i
  • npm i -D @sveltejs/adapter-cloudflare-workers@next and add import cloudflare from '@sveltejs/adapter-cloudflare-workers'; and adapter: cloudflare(), to svelte.config.js
  • wrangler init (assumes you've installed wrangler globally)
  • To the newly created wrangler.toml file add
[build]
upload.format = "service-worker"

or any other wrangler instruction using the . syntax.

  • npm run build <-- Bug throws here

Expected behavior
I expected the build to complete successfully and for me to be able to then publish the built site using wrangler publish.

Information about your SvelteKit Installation:

- @sveltejs/adapter-cloudflare-workers": "^1.0.0-next.8" - @sveltejs/kit: "next"

Severity
Annoying but not a show-stopper presently because I can use an alternate syntax:

[build.upload]
format = "service-worker"

Additional context
The problem is caused by using the stale toml package which hasn't been updated in 2 years and is still back at the 0.4.0 TOML spec. I propose opening a PR to switch to using https://www.npmjs.com/package/@iarna/toml or some other Javascript package (see https://github.com/toml-lang/toml/wiki) which is up to date with the latest TOML spec as done by others to parse the wrangler.toml properly when using TOML syntax introduced after 0.4.0. Can I open a PR?

Someone proposed support for TOML 0.5.0 to the stale toml package, but no dice as the package maintainer is no longer responding: BinaryMuse/toml-node#50

@jacob-8 jacob-8 changed the title Change TOML parser to an active project that supports the latest TOML syntax Change TOML parser to an active project that supports the latest TOML syntax [adapter-cloudflare-workers] May 19, 2021
@benmccann
Copy link
Member

Sounds reasonable to me. @halfnelson wrote the adapter originally so tagging him here in case he wants to weigh in

https://github.com/sveltejs/kit/pull/717/files#diff-43081842b4f8656d01e0b6705906f558acf5f309ea11076f4de04a01d5ecb39fR14

@halfnelson
Copy link
Contributor

Good idea!

@halfnelson
Copy link
Contributor

We should also bump/change the package used in the Netlify adapter too

@jacob-8 jacob-8 changed the title Change TOML parser to an active project that supports the latest TOML syntax [adapter-cloudflare-workers] Change TOML parser to an active project that supports the latest TOML syntax May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants