fix(cli): remove yarn install output and show spinner #236
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/cli/src/commands/add.ts
Outdated
There was a problem hiding this comment.
These errors thrown by GitHub actions were a matter of format/eslint?
There was a problem hiding this comment.
@eunjae-lee, as we discussed in our call, both readFileSync and writeFileSync will be removed in another PR, so there is no need to remove it in this one.
|
I'd love to get another eye on here @BibiSebi :) |
| command, | ||
| { spinnerMessage, ...options } = {}, | ||
| ) => { | ||
| const ora = (await import('ora')).default |
There was a problem hiding this comment.
Is it necessary to import in this way? I suppose there is a reason I am not aware of.
There was a problem hiding this comment.
Very good question!
You know how we support both CJS and ESM with @storyblok/field-plugin.
At packages/field-plugin/package.json, you can see this:
In case of the CLI, we're transpiling our code to CJS only. (But this reminds me that we need to re-research and see if ESM output is good enough for the CLI distribution)
Anyway, if you run yarn build:cli, at packages/cli/dist/main.cjs, you get something like:
...
.... = require('ora');
And this is problematic. The library ora is providing ESM only. Whereas @storyblok/field-plugin provides both CJS and ESM, ora has decided to provide ESM only. There are some library authors who are doing this intentionally, in order to push the JS ecosystem to transit from CJS to ESM more quickly.
Back to require('ora')... so we're requireing the ESM code, which doesn't make sense. That's why it failed. The bundling was okay, but once you run npx field-plugin, node.js will fail to require it with ERR_REQUIRE_ESM error code.
So, how do we solve this issue? I'm doing this:
const ora = (await import('ora')).defaultThis is called "dynamic import". It's different from the topmost static import statements like this:
import ora from 'ora'This dynamic import is supported in Node.js (probably since v14). Although it looks similar, but you can use dynamic import within CJS code. So that's the workaround.
The ideal situation would be for us to bundle our CLI code into ESM (instead of CJS), and we would be able to import ora from 'ora'.
Let me know if anything is unclear!
There was a problem hiding this comment.
Everything super clear, I guess I do not have too much knowledge on the differences between the modules (CJS vs ESM) so I would be glad to support you in the re-research :)
There was a problem hiding this comment.
@BibiSebi done! https://storyblok.atlassian.net/browse/EXT-1885
let's dig later together :)
…g-single-field-plugin
eunjae-lee
deleted the
EXT-1194-hide-yarn-install-output-when-adding-single-field-plugin
branch
What?
createcommand runsnpm install, and we don't have to show its result as long as successful. However, it'd be nice to show a spinner to tell that it's in progress.Why?
JIRA: EXT-1194
How to test? (optional)
Screenshot.2023-07-26.at.11.39.48.mp4