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

[legacy-framework] (newapp) Add .editorconfig and .vscode extension recomendations #2142

Merged
merged 14 commits into from
Apr 2, 2021

Conversation

JuanM04
Copy link

@JuanM04 JuanM04 commented Mar 24, 2021

What are the changes and their implications?

Since VSCode is the most used JavaScript editor, I think we should add recommended extensions and configuration to new apps.

The extensions are:

The default configuration is:

  • Format (with Prettier) on Save
  • Fix ESLint errors on Save

Also, there is a basic EditorConfig!

@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2021

Size Change: +1.61 kB (+1%)

Total Size: 235 kB

Filename Size Change
packages/generator/dist/blitzjs-generator.cjs.dev.js 14.2 kB +534 B (+4%)
packages/generator/dist/blitzjs-generator.cjs.prod.js 14.2 kB +511 B (+4%)
packages/generator/dist/blitzjs-generator.esm.js 13.8 kB +517 B (+4%)
packages/installer/dist/blitzjs-installer.cjs.dev.js 7.52 kB +18 B (0%)
packages/installer/dist/blitzjs-installer.cjs.prod.js 7.52 kB +18 B (0%)
packages/installer/dist/blitzjs-installer.esm.js 7.3 kB +17 B (0%)
ℹ️ View Unchanged
Filename Size Change
packages/babel-preset/dist/blitzjs-babel-preset.cjs.dev.js 1.69 kB 0 B
packages/babel-preset/dist/blitzjs-babel-preset.cjs.js 150 B 0 B
packages/babel-preset/dist/blitzjs-babel-preset.cjs.prod.js 1.69 kB 0 B
packages/babel-preset/dist/blitzjs-babel-preset.esm.js 1.63 kB 0 B
packages/blitz/cli/dist/blitz-cli.cjs.dev.js 1.47 kB 0 B
packages/blitz/cli/dist/blitz-cli.cjs.js 142 B 0 B
packages/blitz/cli/dist/blitz-cli.cjs.prod.js 1.47 kB 0 B
packages/blitz/cli/dist/blitz-cli.esm.js 1.37 kB 0 B
packages/blitz/custom-server/dist/blitz-custom-server.cjs.dev.js 288 B 0 B
packages/blitz/custom-server/dist/blitz-custom-server.cjs.js 149 B 0 B
packages/blitz/custom-server/dist/blitz-custom-server.cjs.prod.js 288 B 0 B
packages/blitz/custom-server/dist/blitz-custom-server.esm.js 123 B 0 B
packages/blitz/dist/blitz.cjs.dev.js 355 B 0 B
packages/blitz/dist/blitz.cjs.js 139 B 0 B
packages/blitz/dist/blitz.cjs.prod.js 355 B 0 B
packages/blitz/dist/blitz.esm.js 102 B 0 B
packages/config/dist/blitzjs-config.cjs.dev.js 1.16 kB 0 B
packages/config/dist/blitzjs-config.cjs.js 146 B 0 B
packages/config/dist/blitzjs-config.cjs.prod.js 1.16 kB 0 B
packages/config/dist/blitzjs-config.esm.js 1.03 kB 0 B
packages/core/config/dist/blitzjs-core-config.cjs.dev.js 260 B 0 B
packages/core/config/dist/blitzjs-core-config.cjs.js 150 B 0 B
packages/core/config/dist/blitzjs-core-config.cjs.prod.js 260 B 0 B
packages/core/config/dist/blitzjs-core-config.esm.js 73 B 0 B
packages/core/dist/blitz-data-080e83b6.cjs.dev.js 1 kB 0 B
packages/core/dist/blitz-data-11f14b56.cjs.prod.js 1 kB 0 B
packages/core/dist/blitz-data-d1f53a00.esm.js 910 B 0 B
packages/core/dist/blitzjs-core.cjs.dev.js 8.69 kB 0 B
packages/core/dist/blitzjs-core.cjs.js 144 B 0 B
packages/core/dist/blitzjs-core.cjs.prod.js 8.42 kB 0 B
packages/core/dist/blitzjs-core.esm.js 8.39 kB 0 B
packages/core/dist/constants-010bc79f.cjs.dev.js 2.92 kB 0 B
packages/core/dist/constants-fbc3a7f6.esm.js 2.82 kB 0 B
packages/core/dist/constants-fcf80a42.cjs.prod.js 2.92 kB 0 B
packages/core/dist/extends-1b905a27.esm.js 241 B 0 B
packages/core/dist/extends-93eedbb0.cjs.dev.js 250 B 0 B
packages/core/dist/extends-f26277ce.cjs.prod.js 250 B 0 B
packages/core/document/dist/blitzjs-core-document.cjs.dev.js 448 B 0 B
packages/core/document/dist/blitzjs-core-document.cjs.js 151 B 0 B
packages/core/document/dist/blitzjs-core-document.cjs.prod.js 450 B 0 B
packages/core/document/dist/blitzjs-core-document.esm.js 272 B 0 B
packages/core/dynamic/dist/blitzjs-core-dynamic.cjs.dev.js 263 B 0 B
packages/core/dynamic/dist/blitzjs-core-dynamic.cjs.js 151 B 0 B
packages/core/dynamic/dist/blitzjs-core-dynamic.cjs.prod.js 263 B 0 B
packages/core/dynamic/dist/blitzjs-core-dynamic.esm.js 73 B 0 B
packages/core/head/dist/blitzjs-core-head.cjs.dev.js 245 B 0 B
packages/core/head/dist/blitzjs-core-head.cjs.js 148 B 0 B
packages/core/head/dist/blitzjs-core-head.cjs.prod.js 245 B 0 B
packages/core/head/dist/blitzjs-core-head.esm.js 64 B 0 B
packages/core/image/dist/blitzjs-core-image.cjs.dev.js 249 B 0 B
packages/core/image/dist/blitzjs-core-image.cjs.js 149 B 0 B
packages/core/image/dist/blitzjs-core-image.cjs.prod.js 249 B 0 B
packages/core/image/dist/blitzjs-core-image.esm.js 65 B 0 B
packages/core/server/dist/blitzjs-core-server.cjs.dev.js 13 kB 0 B
packages/core/server/dist/blitzjs-core-server.cjs.js 148 B 0 B
packages/core/server/dist/blitzjs-core-server.cjs.prod.js 13 kB 0 B
packages/core/server/dist/blitzjs-core-server.esm.js 12.9 kB 0 B
packages/core/with-blitz/dist/blitzjs-core-with-blitz.cjs.dev.js 1.29 kB 0 B
packages/core/with-blitz/dist/blitzjs-core-with-blitz.cjs.js 151 B 0 B
packages/core/with-blitz/dist/blitzjs-core-with-blitz.cjs.prod.js 1.29 kB 0 B
packages/core/with-blitz/dist/blitzjs-core-with-blitz.esm.js 1.15 kB 0 B
packages/display/dist/blitzjs-display.cjs.dev.js 2 kB 0 B
packages/display/dist/blitzjs-display.cjs.js 147 B 0 B
packages/display/dist/blitzjs-display.cjs.prod.js 1.95 kB 0 B
packages/display/dist/blitzjs-display.esm.js 1.86 kB 0 B
packages/file-pipeline/dist/blitzjs-file-pipeline.cjs.dev.js 6.88 kB 0 B
packages/file-pipeline/dist/blitzjs-file-pipeline.cjs.js 150 B 0 B
packages/file-pipeline/dist/blitzjs-file-pipeline.cjs.prod.js 6.88 kB 0 B
packages/file-pipeline/dist/blitzjs-file-pipeline.esm.js 6.61 kB 0 B
packages/generator/dist/blitzjs-generator.cjs.js 148 B 0 B
packages/generator/dist/templates/app/babel.config.js 78 B 0 B
packages/generator/dist/templates/app/blitz.config.js 310 B 0 B
packages/generator/dist/templates/app/jest.config.js 60 B 0 B
packages/installer/dist/blitzjs-installer.cjs.js 148 B 0 B
packages/repl/dist/blitzjs-repl.cjs.dev.js 1.75 kB 0 B
packages/repl/dist/blitzjs-repl.cjs.js 144 B 0 B
packages/repl/dist/blitzjs-repl.cjs.prod.js 1.75 kB 0 B
packages/repl/dist/blitzjs-repl.esm.js 1.6 kB 0 B
packages/server/dist/blitzjs-server.cjs.dev.js 12.6 kB 0 B
packages/server/dist/blitzjs-server.cjs.js 145 B 0 B
packages/server/dist/blitzjs-server.cjs.prod.js 12.6 kB 0 B
packages/server/dist/blitzjs-server.esm.js 12.4 kB 0 B

compressed-size-action

@flybayer
Copy link
Member

Thanks!

I think we should also hide these files from the list of created files printed during blitz new.

@kevinlangleyjr
Copy link
Collaborator

Just a thought 💡

Maybe we should also add an .editorconfig which is usually respected on most IDEs.

@JuanM04
Copy link
Author

JuanM04 commented Mar 26, 2021

I think we should also hide these files from the list of created files printed during blitz new.

I didn't find how to do it without harcoding it in the main Generator class 😕

@flybayer
Copy link
Member

@JuanM04 hardcoding in the generator class is fine I think

@JuanM04
Copy link
Author

JuanM04 commented Mar 26, 2021

@flybayer I sort of did it, but I found that all the contents from .vscode aren't being copied. I've debugging but I don't find the problem. Could you give it a quick look?

@flybayer
Copy link
Member

@JuanM04 ok sweet. I think dot files aren't copied by default. Because we have the below for gitignore in app-generator.

this.fs.move(this.destinationPath("gitignore"), this.destinationPath(".gitignore"))

Maybe there's a way to make it copy dot files by default. Otherwise we have to do the above move trick.

@JuanM04
Copy link
Author

JuanM04 commented Mar 27, 2021

@flybayer what's weird is that fs.readdir doesn't detect .vscode. In fact, it ignore all the "dot directories". However, when you run a simple node script (or run node from the terminal) that reads the template directory, .vscode is listed.

Maybe is something of the local build, but I don't know how to test it

@flybayer
Copy link
Member

@JuanM04 the issue is our build script doesn't copy the contents of .vscode. packages/generator/dist/templates/app/.vscode/ is empty.

See mysticatea/cpx#34

So we need to workaround this somehow. We could use patch-package to patch cpx.

@JuanM04
Copy link
Author

JuanM04 commented Mar 29, 2021

We could use patch-package to patch cpx.

I'll do that. Thank you, I was driving crazy 🤣

@JuanM04
Copy link
Author

JuanM04 commented Mar 30, 2021

@flybayer I just forked cpx (which hasn't been updated since 2016 😮) and added --include-hidden

@flybayer
Copy link
Member

Sweet, thanks!

@flybayer
Copy link
Member

Looks like we have a test failure

@JuanM04
Copy link
Author

JuanM04 commented Mar 31, 2021

I don't know where this error comes from. It "was introduced" in this commit. I'm not sure if I should modify anything, because I could break something.

image

@flybayer
Copy link
Member

flybayer commented Apr 1, 2021

@JuanM04 the failure introduced by that commit is this:

[@blitzjs/cli::jest] FAIL test/commands/new.test.ts (32.455 s)
[@blitzjs/cli::jest]   ● `new` command › when scaffolding new project › pins Blitz to the current version
[@blitzjs/cli::jest] 
[@blitzjs/cli::jest]     TypeError: Cannot read property 'startsWith' of undefined
[@blitzjs/cli::jest] 
[@blitzjs/cli::jest]       1149 | var pluralCamel = /*#__PURE__*/pipe(plural, uncapitalize);
[@blitzjs/cli::jest]       1150 | function camelCaseToKebabCase(transformString) {
[@blitzjs/cli::jest]     > 1151 |   return transformString.replace(/([a-z0-9]|(?=[A-Z]))([A-Z])/g, "$1-$2").toLowerCase();
[@blitzjs/cli::jest]            |                 ^
[@blitzjs/cli::jest]       1152 | }
[@blitzjs/cli::jest]       1153 | 
[@blitzjs/cli::jest]       1154 | var FieldType;
[@blitzjs/cli::jest] 
[@blitzjs/cli::jest]       at AppGenerator.preventFileFromLogging (../generator/dist/blitzjs-generator.cjs.dev.js:1151:17)
[@blitzjs/cli::jest]       at ../generator/dist/blitzjs-generator.cjs.dev.js:763:24
[@blitzjs/cli::jest]           at Array.filter (<anonymous>)
[@blitzjs/cli::jest]       at AppGenerator.run (../generator/dist/blitzjs-generator.cjs.dev.js:759:36)
[@blitzjs/cli::jest]       at New.run (src/commands/new.ts:106:13)
[@blitzjs/cli::jest]       at New._run (../../node_modules/@oclif/command/lib/command.js:43:20)
[@blitzjs/cli::jest]       at whileStayingInCWD (test/commands/new.test.ts:59:13)
[@blitzjs/cli::jest]       at withNewApp (test/commands/new.test.ts:74:13)
[@blitzjs/cli::jest]       at Object.<anonymous> (test/commands/new.test.ts:83:75)

And then this commit 105d720 added another failure which you can solve by mocking out the fs dependencies like we are already doing in packages/generator/test/generators/app-generator.test.ts.

@JuanM04
Copy link
Author

JuanM04 commented Apr 2, 2021

@flybayer I did a small change to packages/installer/src/executors/new-file-executor.tsx. It shouldn't break anything, but since I don't know what it does nor how to trigger it, could you check that file (and/or test it manually) for me?

Edit: I hate you, Windows-node15, why won't you give a green check? Why aren't you more like you brother, Windows-node12?

@flybayer
Copy link
Member

flybayer commented Apr 2, 2021

Yay!!

@flybayer flybayer changed the title VSCode Recommendations (newapp) Add .editorconfig and .vscode extension recomendations Apr 2, 2021
@flybayer flybayer merged commit 63af540 into canary Apr 2, 2021
@flybayer flybayer deleted the 2142-vscode-recommendations branch April 2, 2021 15:12
@dillondotzip dillondotzip changed the title (newapp) Add .editorconfig and .vscode extension recomendations [legacy-framework] (newapp) Add .editorconfig and .vscode extension recomendations Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants