Skip to content
This repository has been archived by the owner on Aug 15, 2023. It is now read-only.

Current WebPack configuration doesn't support TypeScript dependencies #22

Closed
micahswitzer opened this issue May 25, 2020 · 9 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@micahswitzer
Copy link
Collaborator

Because we are directly including the vmmx-schema in this project as a npm dependency, there is no "compilation" down to regular ES5. Because of this, if you try importing any non-type information (enums in my case), webpack will complain that it doesn't understand it.

Example:

import { NoteNames } from "vmmx-schema/notes";

export function getNoteValue(note: string): number {
  return NoteNames[note];
}

Webpack Error:

Failed to compile.

./node_modules/vmmx-schema/note_names.ts 3:7
Module parse failed: Unexpected token (3:7)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| // MIDI defines 0= C-1, 127 = G9
| // 53 = F3, Standard Vibraphone is F3 to F6
> export enum NoteNames {
|       C_1,
|       CS_1,

I don't know what the best way forward is from here, so I'd appreciate some input. For now I'm manually copying the enum over to my code, but this is definitely a hack and needs to be fixed as soon as possible.

@mawi12345
Copy link

I would suggest that all packages are maintained in a single repository (example). This way we can easily import the typescript packages from the workspace. I use lerna for this. If you are interested in restructuring the source code, I can contribute to this. After that it is easy to create new packages and further modularize the code base.

@mozi-h
Copy link
Member

mozi-h commented May 26, 2020

@mawi12345 The reason we split in the first place, was to make it easy for others to import the scheme without the need of importing the whole of virtual-mmx.
We obviously need to import the schema into virtual-mmx – ideally without creating a npm module, whilst keeping it simple and having Intellisense available.

I suppose a monorepo is unavoidable; what would you recommend for the above needs?

@mozi-h mozi-h added the bug Something isn't working label May 26, 2020
@mawi12345
Copy link

mawi12345 commented May 26, 2020

I would use a lerna monorepo and release the packages individually on npm. We use the same approche at work. Users can import the single packages from npm and the development / testing is easier in an monorepo. Also webpack hot reloading across the hole monorepo is possible and saves a lot of time.

@micahswitzer
Copy link
Collaborator Author

micahswitzer commented May 26, 2020

@mawi12345 do you want to take a stab at setting this up? Preserving as much of the original git history would be great, but at this stage I guess it doesn't matter too much.

@Udayraj123
Copy link
Collaborator

Can we not setup ts-loader for it @micahswitzer?
Also, there's an ongoing proposal for a manual webpack configuration, we can include ts-loader there - #26

@Udayraj123
Copy link
Collaborator

Relevant discussion - microsoft/TypeScript#12358

As for the recommendation, publish .js + .d.ts. this way your users do not have to compile your sources each time they need to refer to your library.

@Udayraj123
Copy link
Collaborator

For now, enabled compiling the ts file in usual builds. Slightly increasing dev build times #34

@mozi-h can you verify if this issue can be closed?

@mozi-h
Copy link
Member

mozi-h commented Jun 21, 2020

As far as I get it, #34 closes this; added automatic handling by adding Resolves #22 to #34's description, @Udayraj123.

Udayraj123 added a commit that referenced this issue Jun 22, 2020
changes in webpack.config for issue #22 | tsconfig with explanatory comments,
@Udayraj123
Copy link
Collaborator

Udayraj123 commented Jun 22, 2020

@mozi-h I am guessing it will happen when we merge it into master. I've just merged #34 into dev.
Closing manually as this qualifies as a dev issue. @mozi-h @micahswitzer feel free to reopen

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants