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

chore: add typescript-eslint package scaffold #8296

Merged
merged 1 commit into from Feb 1, 2024

Conversation

bradzacher
Copy link
Member

@bradzacher bradzacher commented Jan 24, 2024

PR Checklist

Overview

This PR just adds an private scaffold for the typescript-eslint package being added in #7935. I've split this out so that the automation will bump the versions with each release.

The most annoying part of the branch is every time I go back to it and rebase it on main I have to manually resolve and correct all of the versions or else things will use old releases from npm instead of the repo code.

@bradzacher bradzacher added the repo maintenance things to do with maintenance of the repo, and not with code/docs label Jan 24, 2024
@typescript-eslint
Copy link
Contributor

Thanks for the PR, @bradzacher!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

Copy link

netlify bot commented Jan 24, 2024

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 7c3f8cc
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/65bb07b2c91994000866ab56
😎 Deploy Preview https://deploy-preview-8296--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 90 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@bradzacher bradzacher force-pushed the core-package-scaffold branch 2 times, most recently from 50df23d to 23cc699 Compare January 24, 2024 04:32
@merceyz
Copy link

merceyz commented Jan 24, 2024

I have to manually resolve and correct all of the versions or else things will use old releases from npm instead of the repo code.

You can replace the explicit versions with workspace:*, Yarn will replace them on publish with the exact version.
https://yarnpkg.com/features/workspaces#cross-references

@bradzacher
Copy link
Member Author

@merceyz oh that's interesting! I didn't know that existed.

I had noticed that there was the syntax when I looked at the lock file (you can see I used it in the root package.json) - but I didn't realise that it would automatically get replaced on publish! That's super cool!

Cc @JamesHenry - we use the nx packages for publishing - does nx use yarn npm publish under the hood or does it not? I.e. Could we use this syntax? Could nx support it?

And for an FYI @JoshuaKGoldberg

@JamesHenry
Copy link
Member

@bradzacher Right now there is a fork in the road when it comes to non semver versions.

Either:

  • you apply your versions using valid semver version in your source files, tracked by git

OR

  • you can leverage either semver versions of any valid workspace/file protocol specifiers if version your dist files, not tracked by git

This second option is how the Nx repo itself does it. The source of truth for versions in this case is therefore not disk, but rather the npm registry. Nx release supports this case via config in nx.json.


Background

This fundamental choice is down to the implementation of nx release being based on isolated version/changelog/publish steps, and the fact that we want to publish something tangible on disk.

Some release tools read your files but then perform all kinds of modifications in memory which are not reflected anywhere on your file system for you to see/play with and then publish that in memory object to npm. This can make troubleshooting bad publishes nearly impossible at times - with this approach it's trivial, because you can always cd somewhere and run npm publish directly with whatever options you want.

So when you perform your versioning step, nx release gets your files ready for your publishing step on disk, therefore it needs to replace those workspace/file references with the real numbers so that they work for your consumers.

Now, having said all that...

When we first created this hard line we only had the three subcommands available: nx release version, nx release changelog, nx release publish and therefore was not straightforward way to plumb things through from one to the other.

These days we have the programmatic API for nx release as a first class use-case - it's already what we use in typescript-eslint, and so it is therefore possible for me to imagine exposing a util function on the programmatic API to reset workspaces/file references after running releasePublish. The main complication though will be not committing those package.json dependency updates as part of the versioning or changelog step. I think it is doable though.

This is the issue to track for progress: nrwl/nx#20978

Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to start, just requesting changes on the license and missing docs. 🚀

packages/core/package.json Outdated Show resolved Hide resolved
packages/core/README.md Outdated Show resolved Hide resolved
@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jan 24, 2024
@github-actions github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 24, 2024
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s finalize the discord discussion before merging

@bradzacher bradzacher changed the title chore: add core package scaffold chore: add typescript-eslint package scaffold Jan 31, 2024
@bradzacher bradzacher force-pushed the core-package-scaffold branch 2 times, most recently from a9ad49e to 7d3db25 Compare February 1, 2024 02:46
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cartoon happy yellow bird bouncing up and down above the word 'YASS'

@bradzacher bradzacher dismissed JamesHenry’s stale review February 1, 2024 09:38

Update name as per discussion.

@bradzacher bradzacher merged commit 94aa28e into main Feb 1, 2024
64 checks passed
@bradzacher bradzacher deleted the core-package-scaffold branch February 1, 2024 09:45
danvk pushed a commit to danvk/typescript-eslint that referenced this pull request Feb 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants