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: update for deno 2 #662

Merged
merged 2 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
run: deno lint

- name: Run tests
run: deno task test
run: deno run test
Copy link
Contributor

@marvinhagemeister marvinhagemeister Sep 6, 2024

Choose a reason for hiding this comment

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

FYI: Changing this is more personal preference. The task command will be the same in Deno 2 as it is today. I guess I'm just trying to get a feel if people like deno run more or if our messaging at Deno sent the wrong signal in that we would be deprecating deno task or something. If so, please let us know 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, what do you think we should have it as

Copy link
Member

@oscarotero oscarotero Sep 7, 2024

Choose a reason for hiding this comment

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

Honesty, I don't like this change in Deno because it opens the door to inconsitencies and makes the API even more complicated and confused than it's currently:

  • deno run can run files and tasks
  • deno ... only can run files but not tasks
  • what does deno run npm:foo run? the npm:foo package or the npm:foo task?
  • I have been eagerly waiting years for this Support deno run resolving bare specifiers with import map denoland/deno#19955 in order to simplify the lume task, but now, if it's merged it will create more inconsistencies. For example, what does deno fmt run? the fmt bare specifier from import maps (if exists) or the fmt command of Deno?

deno task ... and deno run ... were clear and easy to understand. You don't have to do ALL what Node/NPM do.


- name: Test upgrade
run: deno run -A cli.ts upgrade
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ Go to the `v1` branch to see the changelog of Lume 1.
- logging:
- URL transformation direction is more visually distinct. [#563]
- colors replaced to `gray` to support terminals that does not support `dim` colors. [#566]
- `deno task lume upgrade` removes the `deno.lock` file [#527].
- `deno run lume upgrade` removes the `deno.lock` file [#527].
- `transform_images` plugin: don't enlarge images by default [#530].

### Fixed
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ title: Welcome to my page

Build it:

```
deno run -A https://deno.land/x/lume/cli.ts
```sh
deno -A https://deno.land/x/lume/cli.ts
```

This command will compile your documents to HTML and save them into the
Expand Down
7 changes: 1 addition & 6 deletions cli/build_worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,7 @@ onmessage = async (event) => {
site.root(),
);

function mustReload(files: Set<string>): boolean {
if (files.has(_config)) {
return true;
}
return false;
}
const mustReload = (files: Set<string>): boolean => files.has(_config);

watcher.addEventListener("change", (event) => {
const files = event.files!;
Expand Down
8 changes: 2 additions & 6 deletions cli/cms_worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,8 @@ onmessage = async (event) => {
site.root(),
);

function mustReload(files: Set<string>): boolean {
if (files.has(_config) || files.has(_cms)) {
return true;
}
return false;
}
const mustReload = (files: Set<string>): boolean =>
files.has(_config) || files.has(_cms);

site.addEventListener("beforeUpdate", (ev) => {
if (mustReload(ev.files)) {
Expand Down
6 changes: 1 addition & 5 deletions core/loaders/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,5 @@ const binaryFormats = new Set<string>([
]);

export default function getLoader(extension: string): Loader {
if (binaryFormats.has(extension)) {
return binaryLoader;
}

return textLoader;
return binaryFormats.has(extension) ? binaryLoader : textLoader;
}
5 changes: 3 additions & 2 deletions core/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { decodeURIComponentSafe } from "./utils/path.ts";
export interface Options extends Deno.ServeOptions {
/** The root path */
root: string;
port?: number;
}

export const defaults: Options = {
Expand Down Expand Up @@ -92,10 +93,10 @@ export default class Server {
stop() {
try {
this.#server?.shutdown();
} catch (error) {
} catch (err) {
this.dispatchEvent({
type: "error",
error,
error: err as Error,
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export default class FSWatcher implements Watcher {
runningCallback = false;
return watcher.close();
}
} catch (error) {
await this.dispatchEvent({ type: "error", error });
} catch (err) {
await this.dispatchEvent({ type: "error", error: err as Error });
}

runningCallback = false;
Expand Down
7 changes: 5 additions & 2 deletions core/writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,11 @@ export class FSWriter implements Writer {
}</gray>`,
);
return true;
} catch (error) {
log.error(`Failed to copy file: ${file.outputPath}: ${error.message}`);
} catch (error: unknown) {
log.error(
// deno-lint-ignore no-explicit-any
`Failed to copy file: ${file.outputPath}: ${(error as any).message}`,
);
}

return false;
Expand Down
6 changes: 3 additions & 3 deletions deno.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
},
"tasks": {
"test": "TZ=Z LUME_LOGS=ERROR DENO_FUTURE=1 deno test -A",
"test:update": "deno task test -- --update",
"changelog": "deno run --allow-read --allow-write https://deno.land/x/[email protected]/bin.ts",
"update-deps": "deno run -A --quiet 'https://deno.land/x/[email protected]/cli.ts' update deps/*.ts deno.json"
"test:update": "deno run test -- --update",
"changelog": "deno --allow-read --allow-write https://deno.land/x/[email protected]/bin.ts",
"update-deps": "deno -A --quiet 'https://deno.land/x/[email protected]/cli.ts' update deps/*.ts deno.json"
},
"imports": {
"lume/cms/": "https://cdn.jsdelivr.net/gh/lumeland/[email protected]/"
Expand Down
2 changes: 1 addition & 1 deletion init.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { run } from "./deps/init.ts";

console.warn(
"This module is deprecated. Use `deno run -A https://lume.land/init.ts` instead.",
"This module is deprecated. Use `deno -A https://lume.land/init.ts` instead.",
);

run();
2 changes: 1 addition & 1 deletion plugins/decap_cms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const defaults: Options = {
configKey: "decap_cms",
extraHTML: "",
proxyCommand:
`deno run --allow-read --allow-net=0.0.0.0 --allow-write --allow-env ${serverUrl}`,
`deno --allow-read --allow-net=0.0.0.0 --allow-write --allow-env ${serverUrl}`,
};

/**
Expand Down