search

denoland / fresh

The next-gen web framework.
https://fresh.deno.dev
MIT License
12.46k stars 643 forks source link

Doesn't work with vendoring? #596

Closed jimisaacs closed 1 year ago

jimisaacs commented 2 years ago

Not sure, but this doesn't look good?

❯ deno run -A -r https://fresh.deno.dev my-project

Download https://fresh.deno.dev/
Download https://deno.land/x/fresh@1.0.2/init.ts
Download https://deno.land/x/fresh@1.0.2/src/dev/deps.ts
Download https://deno.land/x/fresh@1.0.2/src/dev/error.ts
Download https://deno.land/x/fresh@1.0.2/src/dev/mod.ts
Download https://deno.land/std@0.150.0/flags/mod.ts
Download https://deno.land/std@0.150.0/fs/walk.ts
Download https://deno.land/std@0.150.0/path/mod.ts
Download https://deno.land/std@0.150.0/semver/mod.ts
Download https://deno.land/std@0.150.0/_util/assert.ts
Download https://deno.land/std@0.150.0/fs/_util.ts
Download https://deno.land/std@0.150.0/_util/os.ts
Download https://deno.land/std@0.150.0/path/_interface.ts
Download https://deno.land/std@0.150.0/path/common.ts
Download https://deno.land/std@0.150.0/path/glob.ts
Download https://deno.land/std@0.150.0/path/posix.ts
Download https://deno.land/std@0.150.0/path/separator.ts
Download https://deno.land/std@0.150.0/path/win32.ts
Download https://deno.land/std@0.150.0/path/_constants.ts
Download https://deno.land/std@0.150.0/path/_util.ts
Do you want to use 'twind' (https://twind.dev/) for styling? [y/N] y
Do you use VS Code? [y/N] y
The manifest has been generated for 3 routes and 1 islands.

Project created!

In order to start the development server, run:

$ cd my-project
$ deno task start

❯ cd my-project
❯ deno vendor main.ts
Vendored 83 modules into vendor/ directory.

Updated your local Deno configuration file with a reference to the new vendored import map at vendor/import_map.json. Invoking Deno subcommands will now automatically resolve using the vendored modules. You may override this by providing the `--import-map <other-import-map>` flag or by manually editing your Deno configuration file.

❯ deno run -A --no-remote main.ts
Listening on http://localhost:8000/
✘ [ERROR] [plugin deno] The module was missing and could not be loaded.

An error occurred during route handling or page rendering. Error: Build failed with 1 error:
error: The module was missing and could not be loaded.
    at failureErrorWithLog (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1601:15)
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1243:28
    at runOnEndCallbacks (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1023:63)
    at buildResponseToResult (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1241:7)
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1354:14
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:655:9
    at handleIncomingPacket (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:752:9)
    at readFromStdout (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:621:7)
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1880:11
An error occurred during route handling or page rendering. Error: Build failed with 1 error:
error: The module was missing and could not be loaded.
    at failureErrorWithLog (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1601:15)
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1243:28
    at runOnEndCallbacks (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1023:63)
    at buildResponseToResult (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1241:7)
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1354:14
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:655:9
    at handleIncomingPacket (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:752:9)
    at readFromStdout (file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:621:7)
    at file:///[redacted]/my-project/vendor/deno.land/x/esbuild@v0.14.51/mod.js:1880:11
^C
>>> elapsed time 12s
reggi commented 2 years ago

Hey just dug into this, it's happening because one file https://deno.land/x/fresh@1.0.2/src/runtime/main.ts is used by esbuild here but never imported directly so it's never downloaded as a vendor dependency.

Including it in your vendor deps fixes the issue. 

deno vendor main.ts dev.ts https://deno.land/x/fresh@1.0.2/src/runtime/main.ts

Note: Yourdeno.json inport_map needs to be reset to the original map to NOT use vendor for you to "re-vendor"

reggi commented 2 years ago

As a possible side issue here the vendor map here should be prioritizing the map used in deno.json over the map in the root directory. Right now my fresh instance created with deno task start and vendored fresh is still using the default map internally for esbuild.

sylc commented 2 years ago

Using deno 1.25.4 and fresh 1.1.1, vendoring is failing with the below

$ deno vendor ./main.ts
error: Expected ';', '}' or <eof> at file:///C:/deno-fresh-demo/deno.json:2:10

It seems linked to fact that deno vendor is failing when encountering a type assertion (The fresh manifest is importing deno.json). see tracking issue https://github.com/denoland/deno/issues/15634

sylc commented 2 years ago

The fix for the parsing issue mentioned above is now released on latest deno canary, However there are still a couple more issues with the generated vendor import_maps

$ deno run -A ./main.ts
error: Relative import path "preact/jsx-runtime" not prefixed with / or ./ or ../ and not in import map from "file:///C:/islands/Counter.tsx"

tracking issue: https://github.com/denoland/deno/issues/16108

If the vendor import map is manually adjusted with "preact/jsx-runtime": "https://esm.sh/preact@10.11.0/jsx-runtime" to pass this error, then the twind plugin seems to also be an issue, as below

deno:data:application/javascript,import hydrate from "file:///C:/deno-fresh-demo/vendor/deno.land/x/fresh@1.1.1/plugins/twind/main.ts";import options from "file:///C:/deno-fresh-demo/twind.config.ts";export default function(state) { hydrate(options, state); }:1:20:
      1 │ import hydrate from "file:///C:/deno-fresh-demo/vendor/deno.land/x/fresh@1.1.1/plugins/twind/main.ts";import options from "file:///C:/... 
        ╵                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

An error occurred during route handling or page rendering. Error: Build failed with 2 errors:
error: The module was missing and could not be loaded.
drawveloper commented 1 year ago

@sylc I managed to get exactly to this point, and go beyond by adding the missing deps to the vendor command:

deno vendor --import-map=import_map.json dev.ts main.ts https://deno.land/x/fresh@1.1.2/src/runtime/main_dev.ts https://deno.land/x/fresh@1.1.2/plugins/twind/main.ts

I'd really love to have a fix for "preact/jsx-runtime" not having to be manually added, however.

secext2022 commented 1 year ago

The current workaround (deno 1.35.0, fresh 1.2.0):

> deno vendor main.ts https://deno.land/x/fresh@1.2.0/plugins/twind/main.ts https://esm.sh/preact@10.15.1/jsx-runtime https://deno.land/x/fresh@1.2.0/src/runtime/entrypoints/deserializer.ts https://deno.land/x/fresh@1.2.0/src/runtime/entrypoints/main_dev.ts https://deno.land/x/fresh@1.2.0/src/runtime/entrypoints/signals.ts
Vendored 213 modules into vendor/ directory.

Updated your local Deno configuration file with a reference to the new vendored import map at vendor/import_map.json. Invoking Deno subcommands will now automatically resolve using the vendored modules. You may override this by providing the `--import-map <other-import-map>` flag or by manually editing your Deno configuration file.

then add

    "preact/jsx-runtime": "./esm.sh/preact@10.15.1/jsx-runtime.js"

to vendor/import_map.json

then work ok:

> deno run -A --no-remote main.ts

 🍋 Fresh ready 
    Local: http://localhost:8000/
marvinhagemeister commented 1 year ago

@secext2022 FYI: The missing JSX import source package in the import map was fixed in the newest Deno 1.35.1 release. This is the PR that fixed it https://github.com/denoland/deno/pull/19724

secext2022 commented 1 year ago

ok, now test with deno 1.35.1

> deno --version
deno 1.35.1 (release, x86_64-unknown-linux-gnu)
v8 11.6.189.7
typescript 5.1.6
> deno vendor main.ts https://deno.land/x/fresh@1.2.0/plugins/twind/main.ts https://deno.land/x/fresh@1.2.0/src/runtime/entrypoints/deserializer.ts https://deno.land/x/fresh@1.2.0/src/runtime/entrypoints/main_dev.ts https://deno.land/x/fresh@1.2.0/src/runtime/entrypoints/signals.ts
Vendored 213 modules into vendor/ directory.

Updated your local Deno configuration file with a reference to the new vendored import map at vendor/import_map.json. Invoking Deno subcommands will now automatically resolve using the vendored modules. You may override this by providing the `--import-map <other-import-map>` flag or by manually editing your Deno configuration file.
> deno run -A --no-remote main.ts

 🍋 Fresh ready 
    Local: http://localhost:8000/

nice !

marvinhagemeister commented 1 year ago

Closing because vendoring works with Fresh now. We're in the process of improving the vendor command in Deno in general, but that's not specific to Fresh.

deer commented 1 year ago

Are we sure this is working? In a clean project I tried to do the following:

deno vendor main.ts
deno run -A --no-remote main.ts

and got the following:

Warning "importMap" setting is ignored when "imports" or "scopes" are specified in the config file.
error: A remote specifier was requested: "https://deno.land/std@0.193.0/dotenv/load.ts", but --no-remote is specified.
    at file:///Users/reed/code/temp/vendor_test/main.ts:7:8

I started testing this because I think https://github.com/denoland/fresh/pull/1008 can be closed. Obviously my test didn't go so well, but regardless, the PR seems to encourage a workaround approach, as opposed to getting the core problem fixed.

secext2022 commented 1 year ago

@deer

After deno vendor, you have to remove "imports" section from deno.json file.
And you should also vendor more files, not only main.ts (see comment above)

deer commented 1 year ago

Thanks @secext2022 , that sorted out the remote specifier error. I will create some documentation about vendoring a Fresh project using this information.

@marvinhagemeister, what do you think about closing https://github.com/denoland/fresh/pull/1008 then? It seems like everything is working now.

marvinhagemeister commented 1 year ago

@deer Vendoring is about to change soon to make it less confusing. @dsherret has been doing fantastic work in that direction.

Regarding #1008 : I totally get where they are coming from, but the solution proposed seems more like a hack and potentially confusing for newcomers as to why there are floating dynamic imports. I'd rather make such workaround unnecessarily entirely.

jesperjohansson commented 1 year ago

This worked for me when trying to vendor only fresh: