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

fix: remove import attributes #1117

Merged
merged 10 commits into from
Dec 10, 2024

Conversation

kanadgupta
Copy link
Member

@kanadgupta kanadgupta commented Dec 10, 2024

🧰 Changes

The whole reasoning behind #1115 is because we pass flags in order to avoid emitting this ExperimentalWarning for Node v20 users:

CleanShot 2024-12-10 at 12 12 00@2x

Per this comment, adding -S might be problematic for certain container setups. As a result, I just overhauled how we import JSON in our production code so we don't need that Node CLI flag at all.

At some point I'd love if we got to a point where we can fully rely on the oclif config for package.json attributes but today is not that day!

🧬 QA & Testing

When you check out this branch and run the following commands...

  • Do they run properly without hanging? (hopefully yes)
  • Do you see Node ExperimentalWarning outputs (like the above screenshot) at any point? (hopefully no)
npm ci && npm run build
bin/run.js

@kanadgupta kanadgupta added bug Something isn't working refactor Issues about tackling technical debt labels Dec 10, 2024
@kanadgupta kanadgupta marked this pull request as ready for review December 10, 2024 18:26
Copy link
Contributor

@kellyjosephprice kellyjosephprice left a comment

Choose a reason for hiding this comment

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

Works locally for me

@kanadgupta kanadgupta merged commit e662654 into next Dec 10, 2024
13 checks passed
@kanadgupta kanadgupta deleted the kanad-2024-12-10/remove-import-attributes branch December 10, 2024 19:49
kanadgupta pushed a commit that referenced this pull request Dec 10, 2024
## [9.0.2-next.1](v9.0.1...v9.0.2-next.1) (2024-12-10)

### Bug Fixes

* remove import attributes ([#1117](#1117)) ([e662654](e662654)), closes [#1115](#1115) [/github.com//pull/1115#issuecomment-2532123627](https://github.com//github.com/readmeio/rdme/pull/1115/issues/issuecomment-2532123627)

[skip ci]
@kanadgupta
Copy link
Member Author

🎉 This PR is included in version 9.0.2-next.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

kanadgupta pushed a commit that referenced this pull request Dec 11, 2024
## [9.0.2](v9.0.1...v9.0.2) (2024-12-11)

### Bug Fixes

* **autocomplete:** bad alias ([#1118](#1118)) ([5b8d928](5b8d928))
* remove import attributes ([#1117](#1117)) ([e662654](e662654)), closes [#1115](#1115) [/github.com//pull/1115#issuecomment-2532123627](https://github.com//github.com/readmeio/rdme/pull/1115/issues/issuecomment-2532123627)

[skip ci]
@kanadgupta
Copy link
Member Author

🎉 This PR is included in version 9.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

kanadgupta added a commit that referenced this pull request Dec 11, 2024
@kanadgupta kanadgupta mentioned this pull request Dec 11, 2024
kanadgupta added a commit that referenced this pull request Dec 11, 2024
Reverts #1117 since it's evidently breaking GitHub Actions
workflows 🤡
kanadgupta pushed a commit that referenced this pull request Dec 11, 2024
## [9.0.3-next.1](v9.0.2...v9.0.3-next.1) (2024-12-11)

### Bug Fixes

* revert [#1117](#1117) ([#1119](#1119)) ([c20cc3c](c20cc3c))

[skip ci]
kanadgupta pushed a commit that referenced this pull request Dec 11, 2024
## [9.0.3](v9.0.2...v9.0.3) (2024-12-11)

### Bug Fixes

* revert [#1117](#1117) ([#1119](#1119)) ([c20cc3c](c20cc3c))

[skip ci]
kanadgupta added a commit that referenced this pull request Dec 11, 2024
kanadgupta added a commit that referenced this pull request Dec 11, 2024
## 🧰 Changes

we hit a bit of a wild edge case where the github action was only
breaking in production builds due to how oclif loads the `package.json`
and the only way we caught it was with [this
failure](https://github.com/readmeio/rdme/actions/runs/12267183540/job/34226921877).

this PR reverts #1119 (which in
turn brings back #1117) with a
slight tweak in
f9461b4
to do the following:
- make our import paths friendlier to github actions
- manually copy over the `package.json` to our `dist/` directory
whenever we run `npm run build`. TS was automatically handling this when
we were using JSON imports before but now it's not able to pick up on
the import so we have to copy it over ourselves.

additionally in
669cb4f,
i added a little check so we can catch these sorts of things better
going forward.

## 🧬 QA & Testing

Provide as much information as you can on how to test what you've done.
kanadgupta pushed a commit that referenced this pull request Dec 11, 2024
## [9.0.4-next.1](v9.0.3...v9.0.4-next.1) (2024-12-11)

### Bug Fixes

* bring back [#1117](#1117) without breaking everything ([#1120](#1120)) ([d5d74c5](d5d74c5))

[skip ci]
kanadgupta pushed a commit that referenced this pull request Dec 12, 2024
## [9.0.4](v9.0.3...v9.0.4) (2024-12-12)

### Bug Fixes

* bring back [#1117](#1117) without breaking everything ([#1120](#1120)) ([d5d74c5](d5d74c5))
* **ci:** semantic-release workflow for v9 releases ([#1082](#1082)) ([410daa7](410daa7))
* copy package.json file instead of symlinking ([1d56c21](1d56c21))
* openapi arg doc enhancements, refactors ([#1122](#1122)) ([b83b233](b83b233))

[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor Issues about tackling technical debt released on @next released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants