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

build: stop formatting api files #686

Merged

Conversation

johnhooks
Copy link
Contributor

@johnhooks johnhooks commented Nov 30, 2022

I finally figured out why the *.api.md files kept being reformatted! api-extractor uses the code in the lib folder to build the api files, but it wasn't ignored by Prettier so anytime the script format ran it would format all the files in the lib folders, then on the next run of build:types the reformatted code was used!

Changes in this PR

  • Add lib to .prettierignore
  • Rebuild all the *.api.md files to original output from api-extractor

Now the *.api.md files should only change if the api does.

@vercel
Copy link

vercel bot commented Nov 30, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
dinerojs ✅ Ready (Inspect) Visit Preview Nov 30, 2022 at 8:31PM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 30, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2432bc2:

Sandbox Source
@dinero.js/example-cart-react Configuration
@dinero.js/example-cart-vue Configuration
@dinero.js/example-pricing-react Configuration
@dinero.js/example-starter Configuration

@johnhooks johnhooks force-pushed the build/stop-formatting-api-files branch from 4e50d8e to 2432bc2 Compare November 30, 2022 20:28
@johnhooks johnhooks changed the title build: stop linting and formatting lib build: stop formatting lib Nov 30, 2022
@johnhooks johnhooks changed the title build: stop formatting lib build: stop formatting api files Nov 30, 2022
@sarahdayan
Copy link
Collaborator

Uh! How comes this line didn't work? 🤨

@johnhooks
Copy link
Contributor Author

@sarahdayan because api-extractor must use the declaration files as is and doesn't reformat it.

Steps for issue to happen

  • tsc outputs to lib
  • api-extractor rolls up declarations using code in lib
  • code is committed
  • lib isn't removed because leaving it allows for incremental builds, tsc won't rebuild what hasn't changed.
  • later on someone runs the script format and the lib directory is formatted because it isn't excluded in .prettierignore
  • then someone runs build or build:types
  • tsc runs but the code in src hasn't changed, so it doesn't output new files to lib, they are still the reformatted files.
  • api-extractor runs using the reformatted code, and rolls up the declaration and *.api.md with the code that was reformatted by Prettier!

Does that make sense... it's a pretty convoluted, multistep issue.

@sarahdayan
Copy link
Collaborator

@johnhooks I think what I still don't get is, since Prettier is supposed to ignore all **/*.md files, why it did format them to begin with. No matter where the files are, Prettier should always ignore it, no?

@johnhooks
Copy link
Contributor Author

johnhooks commented Dec 1, 2022

@sarahdayan Prettier didn't touch the *.api.md files, it touched all the lib/**/*.d.ts files that api-extractor used to generate the *.api.md files. We could always clean the lib folders, but we would lose incremental build, but we shouldn't let Prettier build artifacts either, otherwise the problem can happen again when build step are used individually.

Sorry, I tried to be detailed in the explanation before, and must have done a bad job describing the issue.

@johnhooks
Copy link
Contributor Author

@sarahdayan if you want to see exactly what happens.

$ rm -rf packages/*/lib
$ yarn build:types # if your local *.api.md files have been previously modified some of them will have changed
$ git add -A # so you can see what happens next
$ yarn format # watch the output, it formats all the packages/*/lib/**/*.d.ts files, but that didn't modify the git status
$ yarn build:types # *.api.md files have now been generated with the reformatted files

@sarahdayan
Copy link
Collaborator

Ah, gotcha now 👍 I was missing that it's not .md files right away.

@sarahdayan sarahdayan merged commit f130337 into dinerojs:main Dec 1, 2022
@johnhooks johnhooks deleted the build/stop-formatting-api-files branch December 1, 2022 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants