feat: add plot/vega/base/spec2svg-file#13613
Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
| dir = dirname( fpath ); | ||
| if ( !existsSync( dir ) ) { | ||
| mkdir( dir, { | ||
| 'recursive': true |
There was a problem hiding this comment.
This is a modern Node.js API. We'll need to think about how to support in a backward-compatible way.
There was a problem hiding this comment.
Is there a reason why we should be creating directories at all? My sense is that should be done in user land and we shouldn't be guessing what the user wants.
If they want to create a nested directory, they can do that before they ever call this function.
There was a problem hiding this comment.
Agreed, removed the directory auto-creation logic entirely. The output directory is now expected to exist beforehand; if it doesn't, writeFileSync will return an ENOENT error.
| if ( error ) { | ||
| return clbk( error ); | ||
| } | ||
| err = writeFileSync( fpath, svg ); |
There was a problem hiding this comment.
You should be using the async API here. Otherwise, you are mixing async/sync, with the latter blocking the thread.
| * } | ||
| * } | ||
| * | ||
| * spec2svgFile( spec, './output.svg', done ); |
There was a problem hiding this comment.
This is not ideal as, during doctesting, this file will be written and then potentially committed.
In fs/write-file (ref: https://github.com/stdlib-js/stdlib/blob/bd358c950c73dff39b26630b37fe129b5dc11afd/lib/node_modules/%40stdlib/fs/write-file/lib/index.js), we get around this, as we don't recursively make directories, so the examples always error.
Here, if we actually want to write a file, we should use os/tmpdir to write to the host's temporary directory.
That said, I am not convinced that we should be automatically recursively creating directories.
There was a problem hiding this comment.
Yup, Came across a issue where output.svg was being created automatically in root here. Now updated the example to use @stdlib/os/tmpdir so doctesting writes to the host's temporary directory rather than the working directory. Also removed the recursive directory creation entirely.
| * console.log( 'SVG written successfully.' ); | ||
| * } | ||
| */ | ||
| function spec2svgFile( spec, outputPath, clbk ) { |
There was a problem hiding this comment.
Why the argument order here? This differs from fs/write-file where the output filename comes first and then the data.
There was a problem hiding this comment.
No specific reason here. Refactored accoring to fs/write-file.
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves None
Description
This pull request:
plot/vega/base/spec2svg-fileRelated Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
I took assistance from Claude to work on this script. Thought about this approach prior and worked with claude to implement and document.
@stdlib-js/reviewers