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

8.1.0 tsc errors #261

Closed
milesje opened this issue Jan 11, 2021 · 33 comments
Closed

8.1.0 tsc errors #261

milesje opened this issue Jan 11, 2021 · 33 comments

Comments

@milesje
Copy link
Contributor

milesje commented Jan 11, 2021

Upgraded from 7.1.2 to 8.1.0 and now I'm getting the following errors.

[tsc] node_modules/navigo/index.d.ts(31,14): error TS2304: Cannot find name 'ONE'.
[tsc] node_modules/navigo/index.d.ts(31,20): error TS2304: Cannot find name 'ALL'.
[tsc] node_modules/navigo/index.d.ts(44,1): error TS1046: Top-level declarations in .d.ts files must start with either a 'declare' or 'export' modifier.
@milesje milesje changed the title 8.10 tsc errors 8.1.0 tsc errors Jan 11, 2021
@krasimir
Copy link
Owner

Hey @milesje,

can you post here how you are importing Navigo. I mean the actual import statement.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

import Navigo from 'navigo'
and then...
const router = new Navigo('/')

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

I just did a npm update and it is now showing version 8.4.1 and I'm getting the following in my terminal.

[tsc] node_modules/navigo/index.d.ts(41,14): error TS2304: Cannot find name 'ONE'.
[tsc] node_modules/navigo/index.d.ts(41,20): error TS2304: Cannot find name 'ALL'.
[tsc] node_modules/navigo/index.d.ts(55,1): error TS1046: Top-level declarations in .d.ts files must start with either a 'declare' or 'export' modifier.
[tsc] node_modules/navigo/index.d.ts(83,47): error TS2304: Cannot find name 'Router'.
[tsc] src/store.ts(20,43): error TS2554: Expected 1-2 arguments, but got 3.

And I get this in the browser console.

Uncaught SyntaxError: The requested module './../../node_modules/navigo/lib/navigo.js' does not provide an export named 'default'

@krasimir
Copy link
Owner

Are you using some sort of bundler like webpack? I want to replicate the issue on my machine so I can fix it. If you have time setting up an example repo will be the best.

@krasimir
Copy link
Owner

@milesje I managed to replicate the problem. Will provide a fix soon.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

I can upload my package.json file if that will help... but for running it on my PC it does not use a bundler it is using tsc to compile the TypeScript and then uses @web/dev-server to run the compiled code...

package.json is attached as a zip file.
package.zip

krasimir pushed a commit that referenced this issue Jan 12, 2021
@krasimir
Copy link
Owner

Can you try version 8.4.2.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

8.4.2 worked better I'm now only getting 2 errors

[tsc] node_modules/navigo/index.d.ts(55,1): error TS1046: Top-level declarations in .d.ts files must start with either a 'declare' or 'export' modifier.
[tsc] node_modules/navigo/index.d.ts(83,47): error TS2304: Cannot find name 'Router'.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

@krasimir attached is a simplified project that is setup almost identical to my project if you want something to test against. This is a open-wc project using LitElement and a mobx data store.

navigo-test.zip

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

so after applying the PR #264 I no longer get any tsc errors, but instead get an error in the browser console.
Uncaught SyntaxError: The requested module './../../node_modules/navigo/lib/Navigo.js' does not provide an export named 'default'

krasimir pushed a commit that referenced this issue Jan 12, 2021
@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

@krasimir any ideas on the does not provide an export named default error?

@krasimir
Copy link
Owner

Thanks for the PR. Your change is released as 8.4.3 version. Trying to fix the default problem now.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

Thanks for all the help.... I will continue to provide any PR for anything I find that I'm capable of fixing!

@krasimir
Copy link
Owner

Thanks. Also your navigo-test.zip is really useful. I hope I'll resolve this today.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

Let me know if there is anything I can help with....

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

So it looks like webpack is exporting UMD and AMD.... is there a way to get it to export a ESM set as well? I'm not sure why the UMD isn't working, but it might be easier to solve or at least workaround the issue would be to also export a ESM bundle or a CJS bundle.

@krasimir
Copy link
Owner

Yep. I'm preparing a ES build.

krasimir pushed a commit that referenced this issue Jan 12, 2021
@krasimir
Copy link
Owner

Hm ... I just published 8.4.4 which does have an ES export https://github.com/krasimir/navigo/tree/master/lib/es I pointed the module field of the package.json file to the index where we have

exports.__esModule = true;
exports.default = Navigo;

And still I'm getting:

The requested module './../../node_modules/navigo/lib/es/index.js' does not provide an export named 'default'

Weird. I'll leave this for tomorrow morning.

@milesje
Copy link
Contributor Author

milesje commented Jan 12, 2021

Thanks for the update and all the work!

@krasimir
Copy link
Owner

Hey @milesje,

I think I've made some progress on the issue. It looks like the problem is in the web-dev-server. When I changed nodeResolve to false in web-dev-server.config.mjs I'm not seeing this error. Another one pops up tho:

Uncaught TypeError: Failed to resolve module specifier "tslib". Relative references must start with either "/", "./", or "../".

So, I guess there is some tricky part and I'm not sure if the problem is in Navigo since it has now a ES version and that's what is getting used.

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

I don't understand why 7.X works with the current configuration, but not 8.X. I'm 100% sure if is Navigo or some configuration on my part either.

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

So with 8.4.4 and importing navigo/lib/es/index.js I'm unable to perform a npm run build which does not use the web-dev-server. I did have to update the index.html to fix an error there so I've attached that updated index.html file...
index.zip

[!] Error: 'default' is not exported by node_modules\navigo\lib\es\index.js, imported by out-tsc\src\store.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
out-tsc\src\store.js (3:7)
1: import { html } from 'lit-element';
2: import { makeObservable, observable } from 'mobx';
3: import Navigo from 'navigo/lib/es/index.js';

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

I'm wondering if it has to do with the face that you define a class called Navigo (default export in index.d.ts) and then in index.ts you define a default export function called Navigo. I'm not the best at TypeScript or JavaScript, but this seems a little weird to me. Why not just declare the class in index.ts (instead of a function)?

@krasimir
Copy link
Owner

7.X was build and written in a different way. So I'm not surprised that version 8 works differently. As for the class vs function. At this point is not possible to go with a class since I used module-reveal pattern to write the library. Going with a class will mean rewriting everything and I'm a bit skeptical that this is the problem. Will investigate further.

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

Is there a reason to use the module-reveal pattern versus using a class with private members and private functions? A little late now, but just curious....

@krasimir
Copy link
Owner

@milesje back in the days when I started the library there was no classes :) So I decided to continue with the pattern that I used then. Also having a class added a few bytes to the transpiled version.

@krasimir
Copy link
Owner

@milesje I'm trying to run your example without Navigo. When I remove the import and clean up store.ts to not mention the router I'm getting process is not defined. Any idea how to fix it?

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

@krasimir sorry that issue is caused by an update to mobx. To fix it add the following to the index.html file the root directory.

    <script>
      /*global globalThis*/
      globalThis.process = { env: { NODE_ENV: 'development' } };
    </script>

krasimir pushed a commit that referenced this issue Jan 13, 2021
@krasimir
Copy link
Owner

krasimir commented Jan 13, 2021

@milesje I think I fixed it :)

image

Can you try with the latest update 8.6.1

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

@krasimir it does appear to be working... I'm going to run some more advanced scenarios to make sure everything is working... or at least the features I normally use anyway. Thanks a lot for all the hard work! I've really been enjoying this project and hope I can help some more on it.

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

so I just did a small test to add a second route and something isn't working....

this.router
      .on('/', () => {
        this.route = html`<h2>Page Default</h2>
          <br /><a href="/help" data-navigo>help</a>`;
      })
      .on('/help', () => {
        console.log('calling the help page');
        this.route = html`<h2>HELP ME!</h2>`;
      });
    this.router.resolve();

but when I click the help link I'm getting a 'Not Found' page instead of the expected HELP ME
Now I don't normally use href for my site navigation I normally call router.navigate('path...')
But with the configuration I would have expected to be able to go directly to the /help page by going to http://localhost;8001/help but that also results in a 'Not Found' page
The call to router.navigate appears to be working just fine though.

@milesje
Copy link
Contributor Author

milesje commented Jan 13, 2021

never mind.... now it is working as expected but I didn't really do anything other than add a 3rd .on and another href and now they are working as expected.

@vmagalhaes
Copy link

Hello @milesje could you send here an example of how are you using Navigo 8 with WC?

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

No branches or pull requests

3 participants