-
Notifications
You must be signed in to change notification settings - Fork 0
Replace defunct cors-anywhere with tinyurl-rest-wrapper #27
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
Changes from all commits
76fb70e
2270d85
c3204a9
93faa20
703fb1a
1e7efd1
25f8a22
3a4fa67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| name: Test and Publish | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - master | ||
| pull_request: | ||
| branches: | ||
| - master | ||
| release: | ||
| types: [created] | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| node-version: [14, 16] | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Setup Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| cache: 'yarn' | ||
|
|
||
| - name: Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
|
|
||
| - name: Run tests | ||
| run: yarn test | ||
|
|
||
| publish: | ||
| needs: test | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name == 'release' && github.event.action == 'created' | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: '16' | ||
| registry-url: 'https://registry.npmjs.org' | ||
| cache: 'yarn' | ||
|
|
||
| - name: Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
|
|
||
| - name: Build | ||
| run: yarn build | ||
|
|
||
| - name: Publish to NPM | ||
| run: yarn publish --access public --non-interactive | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| node_modules | ||
| dist | ||
| package-lock.json | ||
|
|
||
| .idea | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,15 +1,40 @@ | ||||||||||||||||||||||||||||||||||||||
| import * as axios from "axios"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const shortenUrl = async (url, alias = "") => { | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
2
to
3
|
||||||||||||||||||||||||||||||||||||||
| const shortenUrl = async (url, alias = "") => { | |
| const validateUrl = (url) => { | |
| if (typeof url !== "string" || url.trim() === "") { | |
| throw new TypeError("The 'url' parameter must be a non-empty string."); | |
| } | |
| try { | |
| // This will throw if the URL is not valid. | |
| new URL(url); | |
| } catch { | |
| throw new TypeError("The 'url' parameter must be a valid URL."); | |
| } | |
| }; | |
| const shortenUrl = async (url, alias = "") => { | |
| validateUrl(url); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The README.md documentation still shows that the alias parameter works and will produce a custom alias (line 31: "// https://tinyurl.com/shorted-kulkul"), but the PR description states that alias support has been lost. The documentation should be updated to reflect that the alias parameter is deprecated and no longer functional, or removed entirely to avoid confusing users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shortenUrl() now emits console.warn() whenever an alias is passed. For a library function, this creates unavoidable side effects for consumers (noise in tests/CI logs, potentially breaking apps that treat warnings as failures).
Given alias is intentionally ignored, the more appropriate path is usually either:
- soft-deprecate without logging (document it in README/CHANGELOG), or
- provide an opt-in way to warn (e.g.,
options: { onAliasIgnored }orsilent: true), or - warn only once per process (to limit log spam).
Right now, calling shortenUrl(url, alias) in a loop could flood logs.
Suggestion
Consider removing the unconditional console.warn() and replacing it with an opt-in/once-only mechanism.
Example (warn once):
let warnedAliasIgnored = false;
const shortenUrl = async (url, alias = "") => {
if (alias && !warnedAliasIgnored) {
warnedAliasIgnored = true;
console.warn(
"TinyURL alias parameter is not supported when using tinyurl-rest-wrapper. " +
"The alias will be ignored and a random slug will be generated instead."
);
}
// ...
};Or accept an options object:
const shortenUrl = async (url, alias = "", { warnOnAlias = false } = {}) => {
if (alias && warnOnAlias) console.warn(/* ... */);
// ...
};Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing the once-only warning (or the options-based approach).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignoring
package-lock.jsonis usually counterproductive for libraries/apps that want reproducible installs and deterministic CI. If this repo uses npm, the lockfile should typically be committed rather than ignored.If you intentionally don’t want lockfiles, consider also ignoring
yarn.lock/pnpm-lock.yamlfor consistency and documenting the policy in the README.Suggestion
Decide on a lockfile policy:
package-lock.jsonfrom.gitignoreand commit it.Reply with "@CharlieHelps yes please" if you’d like me to add a commit reverting the
.gitignorechange (and optionally adding a short note to the README about lockfile policy).