dose not work...

[jimmywarting]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure it has not already been reported

Version

n/a

Node.js version

n/a

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

n/a

Description

don't work...

Steps to Reproduce

Expected Behavior

No response

[jimmywarting]

you ask a bounch of unrealistic question like node version and os... this is suppose to run on the browser not on the clients machine... think you should run your test cases in an actual browser instead of running tests cases in node/deno

[fabiancook]

Hi, the questions about node and os are a generic template for the entire virtualstate github org. It is important to know these answers for this project too as it is not purely focused on browser usage, your answers to them is acceptable, I will update the questions to say n/a. (see https://github.com/virtualstate/.github/blob/main/.github/ISSUE_TEMPLATE/bug.yml)

Once browsers include the navigation interface this module will shift into focusing on the server side implementation only most likely.

The error you're seeing looks like it comes from skypack itself rather than something from the implementation. Skypack uses its own bundler and is a bit hard to line up with.

I'll have a look through the issue with skypack and have it resolved.

[fabiancook]

I was sure it was true that I had been testing from within a browser for this project too, I checked the code out and yeah I have playwright tests going for this.

This means the code itself works with the browser, but the bundled version within skypack for whatever reason decided today was not the day. I had previously used skypack fine with this.

[fabiancook]

I can see specifically that for whatever reason esinstall (the build tool used by skypack) is getting caught up on this line here:

Which is usage of a private getter. It may be that I have to drop skypack from the default recommendation for this module, it previously had built just fine!

[fabiancook]

class Cls {
  get #url() {
     return "https://example.com";
  }

  doSomething() {
     const url = new URL(this.#url);
     console.log(url.hostname);
  }
}

const cls = new Cls();
cls.doSomething();

This is a small reproduction of the issue, it works in my browser + node

I couldn't see any issues in snowpack that mention a private getter sadly so it might be something a bit new for snowpack.

I am attempting to remove just that specific file from the published build, which is built only for internal tests (funnily enough for browsers that don't support esm!), I have a feeling not having esm code in a file has esinstall not working properly, who knows what the actual root cause is...

[fabiancook]

I got rid of that error and get a new one...

console.warn("[Package Error] \"@virtualstate/navigation@v1.0.1-alpha.102\" could not be built. \n[1/5] Verifying package is valid…\n[2/5] Installing dependencies from npm…\n[3/5] Building package using esinstall…\nRunning esinstall...\nFailed to load node_modules/@virtualstate/navigation/esnext/location.js\n  Unexpected token (36:8) in @virtualstate/navigation/esnext/location.js\nInstall failed.\nInstall failed.");
throw new Error("[Package Error] \"@virtualstate/navigation@v1.0.1-alpha.102\" could not be built. ");
export default null;

Looking at the line it mentions: Unexpected token (36:8) in @virtualstate/navigation/esnext/location.js

I can see this file is built as:

import { NavigationTransitionCommittedDeferred } from "./navigation-transition.js";
export const AppLocationCheckChange = Symbol.for("@virtualstate/navigation/location/checkChange");
export const AppLocationAwaitFinished = Symbol.for("@virtualstate/navigation/location/awaitFinished");
export const AppLocationTransitionURL = Symbol.for("@virtualstate/navigation/location/transitionURL");
const baseUrl = "https://html.spec.whatwg.org/";
/**
 * @experimental
 */
export class NavigationLocation {
    #options;
    #navigation;
    constructor(options) {
        this.#options = options;
        this.#navigation = options.navigation;
        const reset = () => {
            this.#transitioningURL = undefined;
            this.#initialURL = undefined;
        };
        this.#navigation.addEventListener("navigate", () => {
            const transition = this.#navigation.transition;
            if (transition && isCommittedAvailable(transition)) {
                transition[NavigationTransitionCommittedDeferred]
                    .promise
                    .then(reset, reset);
            }
            function isCommittedAvailable(transition) {
                return NavigationTransitionCommittedDeferred in transition;
            }
        });
        this.#navigation.addEventListener("currentchange", reset);
    }
    ancestorOrigins;
    #urls = new WeakMap();
    #transitioningURL;
    #initialURL;
    get #url() {
        if (this.#transitioningURL) {
            return this.#transitioningURL;
        }
        const { currentEntry } = this.#navigation;
        if (!currentEntry) {
            const initialUrl = this.#options.initialUrl ?? "/";
            this.#initialURL = typeof initialUrl === "string" ? new URL(initialUrl, baseUrl) : initialUrl;
            return this.#initialURL;
        }
        const existing = this.#urls.get(currentEntry);
        if (existing)
            return existing;
        const next = new URL(currentEntry.url, baseUrl);
        this.#urls.set(currentEntry, next);
        return next;
    }
    get hash() {
        return this.#url.hash;
    }
    set hash(value) {
        this.#setUrlValue("hash", value);
    }
    get host() {
        return this.#url.host;
    }
    set host(value) {
        this.#setUrlValue("host", value);
    }
    get hostname() {
        return this.#url.hostname;
    }
    set hostname(value) {
        this.#setUrlValue("hostname", value);
    }
    get href() {
        return this.#url.href;
    }
    set href(value) {
        this.#setUrlValue("href", value);
    }
    get origin() {
        return this.#url.origin;
    }
    get pathname() {
        return this.#url.pathname;
    }
    set pathname(value) {
        this.#setUrlValue("pathname", value);
    }
    get port() {
        return this.#url.port;
    }
    set port(value) {
        this.#setUrlValue("port", value);
    }
    get protocol() {
        return this.#url.protocol;
    }
    set protocol(value) {
        this.#setUrlValue("protocol", value);
    }
    get search() {
        return this.#url.search;
    }
    set search(value) {
        this.#setUrlValue("search", value);
    }
    #setUrlValue = (key, value) => {
        const currentUrlString = this.#url.toString();
        const nextUrl = new URL(currentUrlString);
        nextUrl[key] = value;
        const nextUrlString = nextUrl.toString();
        if (currentUrlString === nextUrlString) {
            return;
        }
        void this.#transitionURL(nextUrl, () => this.#navigation.navigate(nextUrlString));
    };
    async replace(url) {
        return this.#transitionURL(url, (url) => this.#navigation.navigate(url.toString(), {
            replace: true
        }));
    }
    async reload() {
        return this.#awaitFinished(this.#navigation.reload());
    }
    async assign(url) {
        await this.#transitionURL(url, (url) => this.#navigation.navigate(url.toString()));
    }
    [AppLocationTransitionURL](url, fn) {
        return this.#transitionURL(url, fn);
    }
    #transitionURL = async (url, fn) => {
        const instance = this.#transitioningURL = typeof url === "string" ? new URL(url, this.#url.toString()) : url;
        try {
            await this.#awaitFinished(fn(instance));
        }
        finally {
            if (this.#transitioningURL === instance) {
                this.#transitioningURL = undefined;
            }
        }
    };
    [AppLocationAwaitFinished](result) {
        return this.#awaitFinished(result);
    }
    #awaitFinished = async (result) => {
        this.#initialURL = undefined;
        if (!result)
            return;
        const { committed, finished } = result;
        await Promise.all([
            committed || Promise.resolve(undefined),
            finished || Promise.resolve(undefined)
        ]);
    };
    #triggerIfUrlChanged = () => {
        const current = this.#url;
        const currentUrl = current.toString();
        const expectedUrl = this.#navigation.currentEntry.url;
        if (currentUrl !== expectedUrl) {
            return this.#transitionURL(current, () => this.#navigation.navigate(currentUrl));
        }
    };
    /**
     * This is needed if you have changed searchParams using its mutating methods
     *
     * TODO replace get searchParams with an observable change to auto trigger this function
     */
    [AppLocationCheckChange]() {
        return this.#triggerIfUrlChanged();
    }
}
//# sourceMappingURL=location.js.map

Line 36 is the same line it is tripping up on with the rollup file that I have now excluded.

So it seems like private getters just aren't supported with esinstall which is a bit of a shame.

Its the only usage of a private getter in the entire implementation so I will swap it out

[fabiancook]

That was it, it now works as expected with skypack

const { Navigation } = await import("https://cdn.skypack.dev/@virtualstate/navigation@^v1.0.1-alpha.107");

const navigation = new Navigation();

navigation.addEventListener("navigate", async ({ destination }) => {
    if (new URL(destination.url).pathname === "/disallow") {
        throw new Error("No!");
    }
});

console.log(await navigation.navigate("/allowed").finished); // Resolves
await navigation.navigate("/disallow").finished; // Rejects

[jimmywarting]

ty, it works with skyoack now, fyi, before i made the issue i also tried jsfiddle (another variant)

import('https://cdn.jsdelivr.net/npm/@virtualstate/navigation@1.0.1-alpha.95/esnext/index.min.js')

it yielded another error:

Uncaught (in promise) TypeError: Failed to resolve module specifier "abort-controller". Relative references must start with either "/", "./", or "../".

guess u need to have some import map stuff to use it also from some cdn... it's a bit questionable if you should be pulling in abort-signal polyfill also... i would rather want to include a such polyfill myself if i need it (i don't support IE anymore) and all other browser already have good support for abortcontroller nowdays maybe some conditional import with top level await could work...

[fabiancook]

Awesome!

You are correct with the import maps, for the tests I am doing within a browser + deno I am configuring an import map, which includes abort-controller and uuid, these are both newer dependencies.

I had swapped them both out for global versions if they exist.

https://github.com/virtualstate/navigation/blob/e12f3e2665053501705a336172b20e4a861d6815/src/import-abort-controller.ts#L1-L8

https://github.com/virtualstate/navigation/blob/e12f3e2665053501705a336172b20e4a861d6815/src/util/import-uuid.ts#L1-L8

[jimmywarting]

instead of using uuid then i can recommend you to use crypto.randomUUID() that also returns a uuid v4 string (it's synchronous)

and exist in all context

edit: nevermind saw that you already do that

[fabiancook]

The usage of import maps has been removed in favour of AbortController + crypto.randomUUID as part of https://github.com/virtualstate/navigation/issues/7