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

The extension built from sources doesn't work #17

Closed
FluorescentHallucinogen opened this issue Nov 27, 2023 · 20 comments · Fixed by #19 or #20
Closed

The extension built from sources doesn't work #17

FluorescentHallucinogen opened this issue Nov 27, 2023 · 20 comments · Fixed by #19 or #20
Assignees

Comments

@FluorescentHallucinogen
Copy link
Contributor

Environment:

OS: Windows 11
Node.js: 20.10.0

Steps to reproduce:

  1. Clone the repo.
  2. Install chomp globally (npm i -g chomp).
  3. Run chomp build.
  4. Set $JSPM_VSCODE_PATH variable.
  5. Run chomp test.
  6. Run JSPM: Generate Import Map in VSCode.

Actual behavior:

Receiving an

Activating extension 'JSPM.jspm-vscode' failed: TypeError: Cannot destructure property 'numericLiteral' of 'p$N.types' as it is undefined.
  at eval (extension.js:4067:247)
  at R.vb (extensionHostWorker.js:91:627)
  at async Promise.all (index 0)
  at async f.n (extensionHostWorker.js:79:6207)
  at async f.m (extensionHostWorker.js:79:6170)
  at async f.l (extensionHostWorker.js:79:5627)

error.

Expected behavior:

The extension works.

Notes:

Also, I've tried to build the code from 0.2.1 tag instead of the latest code from master branch. The same result.

@FluorescentHallucinogen
Copy link
Contributor Author

@FluorescentHallucinogen
Copy link
Contributor Author

I've also tried to build on macOS instead of Windows. The same result. The output /dist/extension.js file doesn't work in VSCode.

@guybedford
Copy link
Member

Okay, it looks like the Babel build is currently not working through the jspm rollup plugin. We should upgrade to the new JSPM CLI workflow, do an update of the dependencies, and update to a newer version of the build process based on current import-map-based process.

That we don't have an import map in the first place is why the version lock broke and this broke.

This is a top priority for me to ensure you can land your work and we can release updates to this plugin. I hope to get a fix landed on Sunday during my open source hours.

@JayaKrishnaNamburu JayaKrishnaNamburu self-assigned this Dec 7, 2023
@JayaKrishnaNamburu
Copy link
Member

JayaKrishnaNamburu commented Dec 7, 2023

Update:
Looking into this, turns out there is some mis-match of @babel/core versions somewhere. Some babel- plugin seems to read types from default export of @babel/core. Which is not present. I am going to generate new versions of the import maps and rebuild the extension build workflow. Will keep posted here 👍

Screenshot 2023-12-08 at 01 31 03
import*as e from "assert";
import*as t from "@babel/core";
import*as r from "@babel/helper-module-imports";
import*as n from "@babel/helper-environment-visitor";
import*as o from "@babel/helper-simple-access";
import a from "./normalize-and-load-metadata.js";
import "path";
import "@babel/helper-validator-identifier";
import "@babel/helper-split-export-declaration";
var s = n;
try {
    "default"in n && (s = n.default)
} catch (e) {}
var i = t;
try {
    "default"in t && (i = t.default)
} catch (e) {}
var l = {};
Object.defineProperty(l, "__esModule", {
    value: true
});
l.default = rewriteThis$1;
var u = s;
var p = i;
const {numericLiteral: c, unaryExpression: d} = p.types;

@guybedford
Copy link
Member

@FluorescentHallucinogen with #19 merged, this should now be building again at least. Please let us know if it's working now for you.

@JayaKrishnaNamburu
Copy link
Member

@guybedford I am going to fix the bug with cli with updating the versions. Right now the command don't work since I added resolutions flag. Will post here once it is done 👍🏽

@JayaKrishnaNamburu
Copy link
Member

All good with the build now. Please let me know if there are still any issues 👍🏽

@JayaKrishnaNamburu
Copy link
Member

JayaKrishnaNamburu commented Dec 9, 2023

To add few more details to the issue for reference. Babel is using a pattern to export @babel/types from @babel/core which they themself added a comment stating it won't be detected in cjs-esm interop checks.
https://github.com/babel/babel/blob/v7.23.3/packages/babel-core/src/index.ts#L16-L19

From 7.23.. versions they stated to import types from @babel/core instead of @babel/types. Which they are doing till previous versions.

Usage of types in @babel/helper-module-transform

7.21.2

import type * as t from "@babel/types";

https://github.com/babel/babel/blob/v7.21.2/packages/babel-helper-module-transforms/src/index.ts#L17

7.23.3

import { template, types as t } from "@babel/core";

https://github.com/babel/babel/blob/v7.23.3/packages/babel-helper-module-transforms/src/index.ts#L2

As the lazy export pattern is not recognisable in interop. The build from jspm CDN that added into the import-map couldn't export types from the @babel/core pacakge.
And so, the extension started to fail in building.

Since this is a kind of breaking pattern change, but published under a minor update. When we are installing for @babel/[email protected] in generator. It's loading latest `@babel/helper-module-transform' with the bug
https://github.com/jspm/generator/blob/main/package.json#L44
You can check the map here.
https://generator.jspm.io/#02JhYGBkzMwtyC8qyU0s0M0qLsjVzU0sSS3KTMzRLc1kcEhKTErN0U/OL0p1MNczMtAzNAIAOUtwxDQA

@guybedford do you think the CDN should handle this type of issue. Or should we let the babel itself aware that the knows interop pattern with issue is causing breaking changes in others ?

Tried to reproduce the same in node@20, and it seems to work. Perhaps the CDN bundler too should support this ?

@FluorescentHallucinogen
Copy link
Contributor Author

Now I get the following error:

c:\Git\jspm-vscode>chomp build
▶ dist/extension.js
Error: Failed to load build config: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'

x dist/extension.js [310.3472ms]
Unable to complete all tasks.

@guybedford
Copy link
Member

This is a Windows-specific issue it seems. @FluorescentHallucinogen do you have WSL or a posix system you can build on? Otherwise we may need to do some Windows support work further here ...

@FluorescentHallucinogen
Copy link
Contributor Author

Unfortunately, not. In general, I can install the WSL. But I truly believe that Windows support would be helpful. Not so much for me, but for other future contributors. This is a very useful extension.

BTW, I've added JSPM tools to https://github.com/WICG/import-maps#community-polyfills-and-tooling. See WICG/import-maps#297. 😉

@FluorescentHallucinogen
Copy link
Contributor Author

FluorescentHallucinogen commented Dec 10, 2023

I've also noticed that the size of dist/extension.js file has increased from 3.39 MB to 15.6 MB.

@JayaKrishnaNamburu
Copy link
Member

@FluorescentHallucinogen that could be because of unlined source maps now. Guy released a new update for generator, I will be updating the code extension once more with it. The size will be better now. I will remove the sourcemaps too for the build.

@guybedford
Copy link
Member

I did see that PR, certainly thanks for that - the project hasn't had as good marketing the last couple of years as it should have had, so all mentions are very much welcome.

Definitely we need to support Windows builds, this is being tracked in jspm/jspm-cli#2559. In the mean time, I hope you can at least work around those issues though.

With the merge in #21 things should finally be working with the build size back to normal on non-Windows machines.

@FluorescentHallucinogen
Copy link
Contributor Author

FluorescentHallucinogen commented Dec 14, 2023

Just checked on macOS, and it still doesn't work for me. The extension starts, but fails after the second question "Generate: Select environment conditions:".

The chomp build command produces the dist/extension.js file byte-identical to the dist/extension.js file that is currently committed to the master branch.

Could you please recheck that the dist/extension.js file works in VSCode?

I've tried both, running the chomp test command, and replacing the ~/.vscode/extensions/jspm.jspm-vscode-0.2.1/dist/extension.js file.

Now I get the following error:

workbench.desktop.main.js:sourcemap:769 Error: Invalid status code 401 loading file:///Users/user/Documents/Git/lit-demo/simple-greeting.js. undefined  If you are linking locally against your node_modules folder, make sure that you have all the necessary dependencies installed.

The interesting thing is that jspm link index.html -p nodemodules index.html command works, but it for some reason generates the importmap.json file instead of injecting import map to the index.html. 🤔

@JayaKrishnaNamburu
Copy link
Member

Hey, sure I am going to look on my side and revert back here in few minutes 👍🏽

@JayaKrishnaNamburu
Copy link
Member

I ran the local build in my vscode, and seems to work. Can you share the html file or something to check if there is something with the deps.

https://docs.google.com/document/d/1RoRYO92it6fszm_fGv4bjZu23CsP5ug6d7IHnBaXWtU/edit?usp=sharing

@FluorescentHallucinogen
Copy link
Contributor Author

Sure, here's the repo: https://github.com/FluorescentHallucinogen/jspm-vscode-bug

If I use the unmodified extension just installed from the marketplace, all works like a charm. But if I use the chomp test or replace the ~/.vscode/extensions/jspm.jspm-vscode-0.2.1/dist/extension.js with the https://github.com/jspm/jspm-vscode/blob/main/dist/extension.js file, the extension starts, but fails after the second question.

As for the second problem:

The interesting thing is that jspm link index.html -p nodemodules index.html command works, but it for some reason generates the importmap.json file instead of injecting import map to the index.html.

I've found the problem. I've missed the -o. Here's the correct command: jspm link index.html -p nodemodules -o index.htm.

@JayaKrishnaNamburu
Copy link
Member

I was able to reproduce the same, thanks for sharing more info.

I might need to fine tune the importmap little more. Looks like, we are still loading fetch-native instead of fetch-vscode file in the build. And so the extension is failing to load the files. I will look into it more 👍
I am suspecting this, is the cause for this issue
https://github.com/jspm/jspm-vscode/blob/main/importmap.json#L12

@guybedford
Copy link
Member

guybedford commented Dec 14, 2023 via email

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