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

Move Langium to ESM #905

Closed
msujew opened this issue Feb 8, 2023 · 6 comments · Fixed by #1125
Closed

Move Langium to ESM #905

msujew opened this issue Feb 8, 2023 · 6 comments · Fixed by #1125
Labels
repo Monorepo configuration
Milestone

Comments

@msujew
Copy link
Contributor

msujew commented Feb 8, 2023

With the general available ESM support in all major browsers and Node.js, we should consider moving to ESM ourselves.

One of the reasons we've shied away from that was the lack of ESM support in the VSCode extension host. As VSCode is starting to migrate to ESM, we should move along.

@msujew msujew added the repo Monorepo configuration label Feb 8, 2023
@spoenemann
Copy link
Contributor

I assume that would be for version 2.0?

@msujew
Copy link
Contributor Author

msujew commented Feb 9, 2023

Yes, we should try to target this for a version 2.0 of Langium. It shouldn't be too much work, but we need to ensure that it's supported in vscode/theia.

@spoenemann spoenemann added this to the v2.0.0 milestone Feb 9, 2023
trusktr added a commit to bytescript/bytescript that referenced this issue May 27, 2023
@trusktr
Copy link

trusktr commented May 27, 2023

On the user side, I tried to convert bytescript type:module in package.json. I was able to get the cli to work (the project is bootstrapped from the hello-world example, and follows that setup). The cli generator loads and uses Langium APIs fine.

Here's what I did:

The first problem is easy to fix: import specifiers should have file extensions appended to file names, f.e. import {} from './some/file.js'.

F.e. based on the hello-world template, in the generated/ folder after npm run langium:generate, the output should have .js extensions like so:

import { ByteScriptAstReflection } from './ast.js';
import { ByteScriptGrammar } from './grammar.js';

For testing, I modified these generated files by hand after npm run langium:generate.

Then the bin/cli.js file needs to be like so (based on the default hello-world template):

#!/usr/bin/env node

import run from '../out/cli/index.js'
run()

Package.json needs something like the following with the extension included:

	"bin": {
		"bytescript-cli": "./bin/cli.js"
	},

Then tsconfig.json needs module: "esnext".

I then went through all the project TS files and added all the .js extensions to the imports.

The vscode-uri import had to be changed to:

import * as vscodeUri from 'vscode-uri'
const {URI} = vscodeUri

Lastly, in cli/index.ts, I removed the use of the require() for the package.json and replaced it with an fs.readFileSync() at the top of the module.

import fs from 'fs'
const relative = (path: string, base: string) => new URL(path, base).href.split('file://')[1]
const pkgFile = relative('../../package.json', import.meta.url)
const pkg = JSON.parse(fs.readFileSync(pkgFile).toString())

The CLI's generate function works.

I pushed this example to this branch:

https://github.com/bytescript/bytescript/tree/langium-issue-905

including the generated files committed, which are normally gitignored.

After cloning and checking out that branch, running npm install && npm test will output a .wasm file in a top-level generated/ folder, and then run the module in both Node.js and headless Chrome.


However at this point, trying to run the extension for debugging (F5) will result in an error when VS Code's source code tries to require() the extension.js file which is an ES Module.

So VS Code will have to change this to be an import, and hopefully this isn't an issue:

electron/electron#21457

@trusktr
Copy link

trusktr commented May 27, 2023

I believe the last piece to get this working is to use the wonderful deasync package

https://github.com/abbr/deasync

However, it seems to not be built for Apple silicon yet, so I can't test on my MacBook at the moment.

Essentially replace main in package.json with "main": "./extension.cjs", containing this:

const deasync = require('deasync')

// Make an import wrapper in the callback format that deasync supports.
function importSyncImpl(specifier, callback) {
	import(specifier)
		.then(mod => callback(null, mod))
		.catch(err => callback(err))
}

// Wrap it into a synchronous function.
const importSync = deasync(importSyncImpl)

// This blocks until the import resolves, without `await` (no top level `await` in CommonJS):
module.exports = importSync('./out/extension.js')

Here's a new branch implementing this:

https://github.com/bytescript/bytescript/tree/langium-issue-905-2

After npm install, I'm curious to see if F5 works for you.

If it does, this effectively means that all user code is ESM except for this one file that VS Code require()s directly.

@trusktr
Copy link

trusktr commented May 27, 2023

There are a couple of forks of deasync, @kaciras/deasync and wdeasync, with prebuilt binaries for mac, but it seems to hang for me on this M2 macbook.

Ah well. I suppose waiting for VS Code (Electron) to officially support ESM might be the most viable.

trusktr added a commit to bytescript/bytescript that referenced this issue May 27, 2023
@spoenemann spoenemann removed this from the v2.0.0 milestone Jul 4, 2023
@msujew msujew added this to the v2.0.0 milestone Jul 17, 2023
@msujew
Copy link
Contributor Author

msujew commented Jul 17, 2023

I've readded this to the v2 milestone again. I've been testing out making Langium an ESM library and using esbuild to bundle everything as cjs. This seems to work very well, although it requires all users that want to run Langium in vscode to use a bundler. I think this does make sense though, as I don't want to delay the ESM update any further and using a bundler is necessary for users in the long run anyway.

@msujew msujew modified the milestones: v1.3.0, v2.0.0 Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repo Monorepo configuration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants