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

Cleaned up elements of importing dungeoneerCommon #20

Merged
merged 1 commit into from
Sep 1, 2022

Conversation

Awoogamuffin
Copy link
Owner

After playing around with this for a while I've got dungeoner-common to a more pleasant place. Still not perfect but better.

Looking at how other modules do it, it seems they like to export everything from index. So now the index.ts of dungeoneerCommon looks like this:

export * from "./src/schema/schemaTypes.js";
export * from "./src/schema/dungeoneerSchema.js";
export * from "./src/connection/connectionTypes.js";

And I've put the dungeoneerSchame const in src/schema, which feels logical.

The only thing I don't like is having to put the ".js" extension for the modules to be recognised. Even though the .js files are actually empty for a lot of these because they are only type declarations. Despite this, typescript is able to figure it out (I suppose through the .d.ts files?) and in fact the ts imports fail if these .js exports aren't included. Honestly I'm just a bit confused but whatever.

The nice thing about this way of doing this is that

import { DmRequest, DmResponse } from 'dungeoneer-common/dist/types/src/connection/connectionTypes';

becomes just

import { DmRequest, DmResponse } from 'dungeoneer-common';

Which looks nicer to me.

The main thing is I can put functions and variables wherever I want in imported packages now. I'm still at a bit of a loss, to tell the truth.

@Awoogamuffin Awoogamuffin merged commit f999b93 into main Sep 1, 2022
@Awoogamuffin Awoogamuffin deleted the learningLibrary branch September 1, 2022 16:35
@AlanJMac
Copy link
Collaborator

AlanJMac commented Sep 1, 2022

Cool! The use of index.ts is great - much cleaner imports!

I hadn't looked in detail at the server code before to notice the import paths but yes, the new code matches the usual style I'm used to when importing objects from npm installed modules.

But I have a doubt concerning the index.ts file in dungeoneer-common because specifying the .js extensions looks weird to me too.
e.g. export * from "./src/schema/schemaTypes.js";
Isn't this technically exporting from the transpiled code which index.ts wouldn't know about until it too had been transpiled..?

I see that in vscode ctrl+clicking on the .js path will actually take you to the .d.ts type file.
What error do you get if you remove the .js?

For me the following doesn't seem to complain and the UI and server appear to work in development mode (does it fail at production time perhaps..?):

export * from "./src/connection/connectionTypes";
export * from "./src/schema/dungeoneerSchema";
export * from "./src/schema/schemaTypes";

@Awoogamuffin
Copy link
Owner Author

Yeah those exports work fine for Vs code, but if you look at the dungeoneer-server logs you'll see it doesn't work! Gives me a module not found error

There has to be a way to achieve it without using .js, because I don't see that in any other imported node modules, but I haven't figured it out yet.

This and a few other links point to using js, but again,it feels wrong:

microsoft/TypeScript#41887

@AlanJMac
Copy link
Collaborator

AlanJMac commented Sep 3, 2022

Somehow my development environment had gotten corrupted but in the end I also saw that the dungeoneer-server doesn't work if the extensions are not specified.

After some digging around it appears that in a Node project which uses ECMAScript modules, rather than the older CommonJS type, then it is indeed necessary to specify the file extensions in import statements:
https://nodejs.org/api/esm.html#esm_mandatory_file_extensions
The argument for this is that Typescript doesn't modify the import URIs and therefore you have to specify the filenames that will appear in the output javascript files after compilation. So Typescript has been designed to figure this out at compile time and map the source to the relevant .ts files!
(Personally I'm not a fan of this design because we're talking about files that must be compiled containing relative paths to files that don't exist until after compilation. From a maintainability and future compatibility point of view it is a bit ugly...)

In the Angular project we don't need to specify file extensions because the Angular CLI compiles all the source files into fewer .js files which means in the output all the relative import paths will have been interpreted and modified anyway.
https://stackoverflow.com/questions/70461497/why-do-i-need-to-add-the-js-extension-when-importing-files-in-typescript-but-no

In order to not have to specify the files extensions in imports for a Node project then is to use a bundler like webpack or similar.


For the sake of learning, I checked the source for some of the dungeoneerServer's imported node modules to understand why they may not have specified .js extensions and it looks like each one is different:
e.g.

@Awoogamuffin
Copy link
Owner Author

That's so interesting! I appreciate you doing the deep dive

The whole reason I switched away from commonjs was that angular kept complaining that it wouldn't allow tree shaking on that module. Switching to ecmascript was the reason I couldn't get jasmine to work (it worked fine in commonjs)

So yeah, I suppose webpack does a lot of stuff that allows angular to a) use jasmine and b) import dingeoneer common without the .js extensions

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

Successfully merging this pull request may close these issues.

2 participants