jvilk / BrowserFS

BrowserFS is an in-browser filesystem that emulates the Node JS filesystem API and supports storing and retrieving files from various backends.
Other
3.06k stars 216 forks source link

Build issues creating the dist #361

Closed AntonOfTheWoods closed 10 months ago

AntonOfTheWoods commented 11 months ago

From recent commits it appears that esbuild is now used for part of the building and dist package building. However, the built files from the build phase seem to now go directly to dist, rather than build. The make_dist command expects them in build, and fails if they are not. The make_dist command cleans dist before trying to copy there, meaning the files that were just built and put directly in dist are deleted before being copied (though they don't need to be copied...).

james-pre commented 11 months ago

make_dist was used with the old build system and will be removed soon.

AntonOfTheWoods commented 11 months ago

Thanks. How are we supposed to build? Presumably we still need the shims, and I can't see anywhere else they are built.

If they aren't necessary anymore, is the some documentation somewhere that explains how to use the new version? Thanks!

james-pre commented 11 months ago

Just do npm run build. The build script handles everything.

zardoy commented 10 months ago

Hmm, but how do we use this package without shims from make_dist then? Even readme on your fork still references webpack alias usage, why did you remove them (especially fs shim)? I want to use FileSystemAccess backend that is missing on the npm. Probably I'm missing something, but it is unclear to me how to migrate to new version

james-pre commented 10 months ago

@zardoy You don't need to use a shim.

Run the commands:

git clone https://github.com/jvilk/BrowserFS.git
cd BrowserFS
npm i
npm run build

Then, add <script src="/your/path/to/bjs"></script> followed by a <script type="module">...</script>. In the second, you can do something like this:

await BrowserFS.configure({ fs: 'FileSystemAccess', options: { /*  FileSystemAccess options */ } });
const fs = BrowserFS.BFSRequire('fs');
AntonOfTheWoods commented 10 months ago

@zardoy , I'm pretty sure we do still need to use shims if we want to use with an existing codebase that thinks it is using node. My advice would be to look at what was done before and make your own shims, and then simply point to you own rather than browserfs provided shims.

james-pre commented 10 months ago

It should work without shims. If it does not, please create an issue for the specific feature that is missing (assuming an issue to add it does not already exist).

zardoy commented 10 months ago

It should work without shims. If it does not, please create an issue for the specific feature that is missing (assuming an issue to add it does not already exist).

I was just reading the body of https://github.com/jvilk/BrowserFS/commit/7ffedfd9139fb97b3b3aea6d937e9e4eefcc1868 (Removed uneeded polyfills) and wanted to point out that these shims are actually needed in most cases (I still believe providing an alias for fs is a quite common case), so, yes, I think it would be nice if you could bring them back for existing webpack configs like this.

simply point to you own rather than browserfs provided shims

probably this would be okay since they are pretty small, though I still can't figure out how to make new version work with webpack (I see that it works with native imports)

Btw with the new build system, it seems there is no longer a way to use the non-minified versions (because of exports field).

AntonOfTheWoods commented 10 months ago

It should work without shims. If it does not, please create an issue for the specific feature that is missing (assuming an issue to add it does not already exist).

I was waiting for a release so we could better understand your vision for the project.

james-pre commented 10 months ago

@zardoy @AntonOfTheWoods Shims are uneeded. If you want to require and package from BrowserFS (i.e. fs) You can use resolve for Webpack, @rollup/plugin-alias for Rollup, etc. BrowserFS should not be used for path, buffer, etc. polyfill. It is meant for providing fs, nothing else.