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

Add standalone CLI #14270

Merged
merged 31 commits into from
Sep 2, 2024
Merged

Add standalone CLI #14270

merged 31 commits into from
Sep 2, 2024

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented Aug 27, 2024

This PR adds a new standalone client: A single-binary file that you can use to run Tailwind v4 without having a node setup. To make this work we use Bun's single-binary build which can properly package up native modules and the bun runtime for us so we do not have to rely on any expand-into-tmp-folder-at-runtime workarounds.

When running locally, pnpm build will now standalone artifacts inside packages/@tailwindcss-standalone/dist. Note that since we do not build Oxide for other environments in the local setup, you won't be able to use the standalone artifacts for other platforms in local dev mode.

Unfortunately Bun does not have support for Windows ARM builds yet but we found that using the bun-baseline runtime for Windows x64 would make the builds work fine in ARM emulation mode:

Screenshot windows

Some Bun related issues we faced and worked around:

  • We found that the regular Windows x64 build of bun does not run on Windows ARM via emulation. Instead, we have to use the bun-baseline builds which emulate correctly.

  • When we tried to bundle artifacts with embed directories, node binary dependencies were no longer resolved correctly even though they would still be bundled and accessible within the embeddedFiles list. We worked around this by using the import * as from ... with { type: "file" }; and patching the resolver we use in our CLI.

  • If you have an import to a module that is used as a regular import and a with { type: "file" }, it will either return the module in both cases or the file path when we would expect only the with { type: "file" } import to return the path. We do read the Tailwind CSS version via the file system and require.resolve() in the CLI and via import * from './package.json' in core and had to work around this by patching the version resolution in our CLI.

    import packageJson from "./package.json"
    import packageJsonPath from "./package.json" with {type: "file"}
    
    // We do not expect these to be equal
    packageJson === packageJsonPath 
  • We can not customize the app icon used for Windows .exe builds without decompiling the binary. For now we will leave the default but one workaround is to use tools like ResourceHacker to decompile the binary first.

@philipp-spiess philipp-spiess changed the title Feat/v4 standalone cli workaround Add standalone CLI Aug 27, 2024
@philipp-spiess philipp-spiess force-pushed the feat/v4-standalone-cli-workaround branch 2 times, most recently from 5e567f6 to 2ac8a71 Compare August 29, 2024 11:22
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
scripts/pack-packages.mjs Show resolved Hide resolved
@philipp-spiess philipp-spiess marked this pull request as ready for review August 29, 2024 12:05
@philipp-spiess philipp-spiess force-pushed the feat/v4-standalone-cli-workaround branch from ceed332 to 8db7e61 Compare August 29, 2024 12:42
@philipp-spiess philipp-spiess force-pushed the feat/v4-standalone-cli-workaround branch from 8db7e61 to 29125f0 Compare August 29, 2024 12:55
@Jarred-Sumner
Copy link

Very exciting - and thanks for the feedback.

We found that the regular Windows x64 build of bun does not run on Windows ARM via emulation. Instead, we have to use the bun-baseline builds which emulate correctly.

This is unfortunately expected, and we should just document it. arm64 emulation (including Rosetta) tends not to support SIMD instructions

If you have an import to a module that is used as a regular import and a with { type: "file" }, it will either return the module in both cases or the file path when we would expect only the with { type: "file" } import to return the path. We do read the Tailwind CSS version via the file system and require.resolve() in the CLI and via import * from './package.json' in core and had to work around this by patching the version resolution in our CLI.

Currently, we treat file paths as the key in some hash tables, but it really should be file path and loader so that this works as expected

Internally, bun build --compile does try to make fs.readFile and Bun.file(path).{text,arrayBuffer,json,bytes} support embedded files without extra code changes, but I don't think we're doing a good enough job of this yet

When we tried to bundle artifacts with embed directories, node binary dependencies were no longer resolved correctly even though they would still be bundled and accessible within the embeddedFiles list. We worked around this by using the import * as from ... with { type: "file" }; and patching the resolver we use in our CLI.

This is a bug. It's probably using the "file" loader instead of our "napi" loader. Would you be able to file an issue?

We can not customize the app icon used for Windows .exe builds without decompiling the binary. For now we will leave the default but one workaround is to use tools like ResourceHacker to decompile the binary first.

We can add a --icon CLI flag or similar.

// with bun, some of the ids might resolve to files inside the bun
// embedded files root which can only be read by `node:fs` and not
// `node:fs/promises`.
return readFileSync(id, 'utf-8')

Choose a reason for hiding this comment

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

This seems like a bug in Bun.


async function buildForPlatform(triple: string, outfile: string) {
// We wrap this in a retry because occasionally the atomic rename fails for some reason
for (let i = 0; i < 5; ++i) {

Choose a reason for hiding this comment

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

Definitely a bug in Bun

Is this happening on all platforms or on a specific platform?

Copy link
Contributor

Choose a reason for hiding this comment

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

When I ran into this it was on macOS. Not sure if it applies to other platforms. It happened enough that I added a loop for it but it was very inconsistently reproducible.


await buildForPlatform(triple, outfile)

await new Promise((resolve) => setTimeout(resolve, 100))

Choose a reason for hiding this comment

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

Was this delay a workaround of a bug?

Copy link
Contributor

@thecrypticace thecrypticace Aug 30, 2024

Choose a reason for hiding this comment

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

It was because the readFile that follows would occasionally fail unless we waited a little bit. Perhaps it's like a filesystem buffer syncing thing?

I need to give it another test to see if it's still a problem.

return localResolve(id)
}

const resolver = EnhancedResolve.ResolverFactory.createResolver({

Choose a reason for hiding this comment

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

It's probably not feature-ful enough, but you might be able to instead use Bun.resolveSync here. Bun.resolveSync supports a 2nd argument for the path.

You can see some tests for it here:
https://github.com/oven-sh/bun/blob/6603133732892227f0a32e3f19b343fd1e59a604/test/js/bun/resolve/import-meta.test.js#L133-L154

Copy link
Contributor

@thecrypticace thecrypticace Aug 30, 2024

Choose a reason for hiding this comment

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

Unfortunately I don't think we can because we need support for the style field and style conditions in exports. Good to know about this though — perhaps it could be usable elsewhere.

@thecrypticace
Copy link
Contributor

This is unfortunately expected, and we should just document it. arm64 emulation (including Rosetta) tends not to support SIMD instructions

FWIW, Rosetta 2 translates SSE2 instructions in Sonoma at least. In Sequoia it'll also support translation of AVX2 instructions (I assume that includes normal AVX as well). Translation of AVX512 is not supported tho.

Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

This seems to work on my machine as expected. Only thing I want to figure out is to extend the integration tests to also test using the standalone CLI.

We currently only test on Linux and Windows, so we don't fully test every single OS that we generate binaries for. But if those two are tested via integration tests already that would be sick.

@philipp-spiess philipp-spiess merged commit d9e3fd6 into next Sep 2, 2024
3 checks passed
@philipp-spiess philipp-spiess deleted the feat/v4-standalone-cli-workaround branch September 2, 2024 13:23
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.

4 participants