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

What is the proper way of importing modules in typescript (ECMAScript 2015)? #199

Open
UndeadKernel opened this issue Nov 30, 2022 · 14 comments
Labels

Comments

@UndeadKernel
Copy link

UndeadKernel commented Nov 30, 2022

When using ECMAScript 2015, import statements are lexically bound. That is, everything inside a module must be either declared or imported by a module. There is no such thing as globals outside of the module.

The modules of jsPanel assume that the jsPanel object exists in the global scope. Due to this, the following lines of code result in an error:

import jsPanel from "dist/jspanel.js"
import "dist/extensions/modal/jspanel.modal.js"

The last import triggers an error because jsPanel is not defined.

What is the appropriate way of using modules in this context?
For reference, I'm building a web application with vue3 and typescript.

@Flyer53
Copy link
Owner

Flyer53 commented Dec 1, 2022

In your code example above you use the files from the dist folder. They don't export anything and thus don't work as modules. In order to use jsPanel as js module you have to use the files from the es6module folder. And don't forget to use type="module" in the script tag.

<script type="module">
    import { jsPanel } from './es6module/jspanel.js';
    import './es6module/extensions/modal/jspanel.modal.js';
    .
    .
    .

Please try that first and let me know whether it works.

PS: I don't use vue and/or typescript at all. So I won't be able to help you with any questions about vue or typescript.

@Flyer53
Copy link
Owner

Flyer53 commented Jan 6, 2023

Did you try anything concerning this issue? It's been more than a month now ...

@UndeadKernel
Copy link
Author

Sorry for the delay @Flyer53. I'm still trying to make JsPanel work with the modules syntax. I'll report back when I find what is causing me problems. I suspect it's something related to Vue or Typescript.

By the way, I've been using a typescript data type for JsPanel. Would you be interested in a type declaration for JsPanel so that it can be used with Typescript?

@Flyer53
Copy link
Owner

Flyer53 commented Jan 16, 2023

@UndeadKernel
No problem at all, sorry that I can't help you with Vue or Typescript issues.

Although I won't start using Typescript I could add your type declaration to the repo in case it's usable for everybody else. I would name you as originator of the type declaration for any further questions on this topic.

@UndeadKernel
Copy link
Author

I was able to get everything running with the explanation you gave me. Thanks.
Most of the issues were related to the creation of a Typescript module for JPanel. I have done that and I intend to create a pull request with the type definition. Where should I put it in your repo?

@Flyer53
Copy link
Owner

Flyer53 commented Feb 21, 2023

Thanks a lot for your work.
Well, if your Typescript definition is usable for both the module version and the regular version, then I would put the definition file in the root directory. If you can use it only for the module version put it in the es6module folder.

Info: I'm out of town for the next two or three weeks. So it will take some time before I can look at it...

@steddyman
Copy link

Hi

I also can't seem to get this working in the context of an ES6 module. I am using it in a chrome extension.

jsPanel alone works fine after installing via npm and installing as follows:

import { jsPanel } from 'jspanel4';

I can then import the contextmenu extension in the following way:

import 'jspanel4/es6module/extensions/contextmenu/jspanel.contextmenu.js';

However, when I then try and use jsPanel.contextmenu.create, I get the following error:
content.js:6170 Uncaught TypeError: Cannot read properties of undefined (reading 'create')

This is how I am referencing the extension

    jsPanel.contextmenu.create({
        target: '#overlay-toggle-button',
        content: `<div class="contextmenu1">
                <a href="#"><i class="fad fa-paste fa-fw"></i>Paste</a>
                <a href="#"><i class="fad fa-cut fa-fw"></i>Cut</a>
                <a href="#"><i class="fad fa-copy fa-fw"></i>Copy</a>
                <a href="#"><i class="fad fa-edit fa-fw"></i>Edit</a>
            </div>`,
        contentSize: 'auto auto',
        callback: (panel) => {
            panel.querySelectorAll('.contextmenu1 a').forEach((a) => {
                a.addEventListener('click', (e) => {
                    e.preventDefault();
                    console.log(a.textContent);
                })
            });
        }
    });

Which is the sample for how to use contextmenu. Without using extensions, I can use jsPanel just fine. It is like the standalone import of the extension is being imported into a different context than the imported jsPanel in the content script that imports jsPanel.

@steddyman
Copy link

steddyman commented Nov 20, 2024

Answering my own question. It seems that the reason I am seeing this problem is because there are two instance of jspanel4 being loaded from two different paths.

If I explicity set the path to the ES6 module, then it works:

import { jsPanel } from 'jspanel4/es6module/jspanel.js';
import 'jspanel4/es6module/extensions/contextmenu/jspanel.contextmenu.js';

Thanks for producing such a great component.

@Flyer53
Copy link
Owner

Flyer53 commented Nov 21, 2024

@steddyman Thanks for using jsPanel, glad you like it 😄
Your solution is basically how the use of jsPanel as module is published in the docs. See https://jspanel.de/#install
Maybe I should add a note concerning the importance of the path to the files.

Question from my side: Is the chrome estension you created publicly available? If so, what kind of extension is it?

Happy coding,
Stefan

@steddyman
Copy link

It's great. If there is something I don't like, I'll submit a PR :)
Yes, it is very similar, but it is not obvious from the docs that if you just import jspanel4, it will have a different instance to the one imported via the extensions, hence need to specify the full path to the es6module.

Yes, the extension is publicly available, but the current version only contains a Chrome Side Panel for the UI. This doesn't allow the extension to be used on mobile devices, since none of them support Side Panels. I have an early beta using an overlay UI over the game view that is using jspanel, and expect to have a full implementation within a few weeks. It is in my discord (link in extension) if you are interested.

The link to the extension is here if you want to see it: https://chromewebstore.google.com/detail/pixels-guru-companion/ddkbonhhkbnghgpgckmaajoephglejkj?hl=en-GB&authuser=0

Problem I am facing now is I want to click a docked icon on the right to have it spring out a contextmenu to select a function. When the function is selected, a specific UI window will open for that function. I've had to hard code the zIndex into the extension because it defaults to only overlaying other jspanel UI's. My button overlays the game so has a zIndex of 2000 which is higher than the menu. The hack works but I might have to circle round and make that neater via a PR.

@Flyer53
Copy link
Owner

Flyer53 commented Nov 22, 2024

@steddyman
I understand your zIndex issue. But you have a very specific use case with very specific needs. The basic jsPanel code can't be perfect for all possible use cases. So I guess hard coding the zIndex might be the preferable solution in your case. At least I don't have a better idea right now ...

@steddyman
Copy link

@Flyer53
Yes, I understand that. Though I would argue if you need to create a floating context menu not attached to another panel, then you would have the same issue since the existing UI could have any zIndex. Perhaps it should have a zIndex override setting for scenarios like this?

If there was a bare version of the panel that could be used for my overlay button

@steddyman
Copy link

steddyman commented Nov 22, 2024

I am seeing a strange issue running the UI on mobile, which I believe is down to the use of the context menu extension.

I note in the the context menu, it sets the container to the BODY tag. I am seeing an Uncaught TypeError: Cannot read properties of null (reading getBoundingClientRect) in the following code (last line):

        // container is either 'window' or the panel's parent element
        const container = panel.options.container === 'window' ? 'window' : panel.parentElement;

        // get base values in order to calculate position deltas
        // since getBoundingClientRect() calculates values relative to the viewport the parentElement of panel/elmtToPositionAgainst is irrelevant
        const panelRect = panel.getBoundingClientRect(),
            containerDomRect = panel.parentElement.getBoundingClientRect(),

Looking at that code, the container property will be set to 'body' from the context menu extension. The body element doesn't have a parentElement property since it is the top element and is therefore throwing an exception.

Perhaps it should instead be setting the container element to window?

Here is the code in the context menu popup that is setting the container type:

                    opts = Object.assign({}, this.defaults, opts, {
                        position: false,
                        container: 'body'
                    });

@steddyman
Copy link

Do you have any idea why I am seeing this exception with the egtBoundingClientRect with parentElement being null?

I tried changing the options.container to window, but it still throws the same error on mobile devices.

I happens when I close a window, then reopen the same window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants