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

Provide npm package via dnt #7

Merged
merged 14 commits into from
Jul 1, 2023
Merged

Provide npm package via dnt #7

merged 14 commits into from
Jul 1, 2023

Conversation

kwaa
Copy link
Contributor

@kwaa kwaa commented Jul 1, 2023

It is currently stuck in the "Print async filters" test of print.test.ts.

undici for node.js does not support file:// url, will get a "not implemented yet" error (nodejs/undici#2144)

Run the dnt build: deno run -A scripts/build_npm.ts <version>

kwaa added 3 commits July 1, 2023 18:07

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@kwaa kwaa changed the title Provide npm package via dnt Provide npm package via dnt Jul 1, 2023
@kwaa
Copy link
Contributor Author

kwaa commented Jul 1, 2023

It looks like the vento package name is already in use. @oscarotero what do you think?

Regarding testing, unfortunately I didn't find the node.js fetch polyfill with file:// support.

Perhaps the dnt test should be disabled directly::

# ./scripts/build_npm.ts
await build({
+ test: false,
  ...
})

@oscarotero
Copy link
Member

Hey, thanks for this.
Regarding the package name, maybe it can use ventojs or something similar. What do you think?
And about the test, I guess it can be disabled for Node environments.

@kwaa
Copy link
Contributor Author

kwaa commented Jul 1, 2023

Hey, thanks for this. Regarding the package name, maybe it can use ventojs or something similar. What do you think?

Okay, looks good.

@kwaa
Copy link
Contributor Author

kwaa commented Jul 1, 2023

Almost done.
I didn't add an Action, but the example at denoland/dnt should work.
(npm packages are untested)

btw, it might be possible to request a vento.js.org domain from js.org
also, can I join the lumeland org? 🥺

@kwaa kwaa marked this pull request as ready for review July 1, 2023 11:15
@oscarotero
Copy link
Member

Sorry, I didn't explain well. I meant disable ONLY this test in Node (but run all other tests). You can use theignore option for that:

Deno.test({
  name: "Print async filters",
  ignore: typeof Deno === "undefined"
  fn: async () => {...}
});

btw, it might be possible to request a vento.js.org domain from js.org

That would be great. I'm a bit busy right now but feel free to request it.

also, can I join the lumeland org? 🥺

Yeah, sure! I just send you an invitation.

@kwaa
Copy link
Contributor Author

kwaa commented Jul 1, 2023

Sorry, I didn't explain well. I meant disable ONLY this test in Node (but run all other tests).

Ok, but typeof Deno is automatically replaced by dnt with typeof dntShim.Deno so it won't work. I replaced it with globalThis?.process?.release?.name === "node",.

That would be great. I'm a bit busy right now but feel free to request it.

Simply put, a CNAME file needs to be created to point to vento.js.org and I can do the rest.
The docs will be temporarily unavailable until the PR is merged.

Yeah, sure! I just send you an invitation.

thx!

@oscarotero oscarotero merged commit 2ccbbd1 into ventojs:main Jul 1, 2023
@oscarotero
Copy link
Member

thanks!
btw, I have added the CNAME file

@kwaa
Copy link
Contributor Author

kwaa commented Jul 1, 2023

btw, I have added the CNAME file

Hmmm... It looks like the github pages for this repository are not deployed in the same way.
The CNAME file needs to appear in the _site folder after the lume build, and the build command needs to be changed to deno task build --location=https://vento.js.org

@oscarotero
Copy link
Member

Ok, done

@kwaa
Copy link
Contributor Author

kwaa commented Jul 1, 2023

Ref js-org/js.org#8345

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.

2 participants