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

fix: generate valid javascript modules in build. fixes #963. #1080

Merged
merged 3 commits into from
Nov 29, 2023
Merged

fix: generate valid javascript modules in build. fixes #963. #1080

merged 3 commits into from
Nov 29, 2023

Conversation

mreinstein
Copy link
Contributor

No description provided.

@mreinstein mreinstein changed the title fix: generate valid javascript modules in build/ fixes #963 fix: generate valid javascript modules in build/ fixes #963 Nov 28, 2023
@mreinstein mreinstein changed the title fix: generate valid javascript modules in build/ fixes #963 fix: generate valid javascript modules in build. fixes #963. Nov 28, 2023
@mreinstein
Copy link
Contributor Author

#963

@jvanbruegge
Copy link
Member

I would strongly prefer something like this instead of basically importing non-existent javascript files

@mreinstein
Copy link
Contributor Author

tsc doesn't seem to have any issues with resolving .js. npm run build and npm run unit work without issue, even with this change:

Screenshot 2023-11-28 at 3 06 22 PM Screenshot 2023-11-28 at 3 07 11 PM

@mreinstein
Copy link
Contributor Author

I would strongly prefer something like

@jvanbruegge done.

@jvanbruegge jvanbruegge merged commit 7ec0e90 into snabbdom:master Nov 29, 2023
1 check passed
@kuraga
Copy link

kuraga commented Nov 29, 2023

@mreinstein , thanks very much!

@mreinstein , @jvanbruegge , some questions, please... As the result:

  1. Are our output files scripts (.js) or modules (.mjs)?
    (Update: oh, .js files and "type": "module" in package.json)
  2. Can we import them as ECMAScript Modules?
  3. Can we require() them as CommonJS files?
  4. Can we import them as TypeScript Modules (in a TypeScript project)?

Thanks!

P.S. https://2ality.com/ has articles about these (the 2022 state).

@mreinstein
Copy link
Contributor Author

mreinstein commented Nov 29, 2023

  1. our output files are es modules, with the .js file extension.
  2. yes, you can import them in a browser as a URL, or in node, provided you have set "type": "module" in your package.json and are using node >= v12.18.x
  3. not directly, but you can use build tooling to handle that if you want it. I don't use legacy cjs anymore, and this project hasn't for a long time either.
  4. I can't speak to

@kuraga
Copy link

kuraga commented Nov 29, 2023

@mreinstein , thanks! Now I see (and yes, this PR doesn't change globally introduced by 09f2d1c and 7af7e3f).

But yeah, it's nice to add an instruction for TypeScript projects...

@iambumblehead
Copy link
Contributor

iambumblehead commented Nov 29, 2023

In the current master branch, npm run build produces a jsx.js file with a bad import.

$ grep \\.\/h build/jsx.js
import { h } from "./h"; // should be "./h.js"

I encountered the issue in another PR and patched the file with sed https://github.com/snabbdom/snabbdom/pull/1084/files#diff-2d40d3ee7ec7a92500d7f2dbb4ab60b274c089a6062d7c5ac0cf9fdc63eddfe1R14


#439

Can we pause on this for a minute? We've already inundated the maintainers with a lot of material today. I'd like to get other existing PRs/issues resolved before we expand out into more discussions.

#957 (comment)

Possibly, but I'm waiting for feedback from Jan. There's already a lot of stuff flying around today. I don't want to jam more activity in until he's had a chance to catch up.

These things don't feel good: "unpausing/unrelenting", "inundating", "preemptively expanding" and "jamming"

Please allow me to comment without casting such things and allow others to represent their own thoughts to the rest.

@mreinstein
Copy link
Contributor Author

You pinged me and specifically asked for suggestions:

mreinstein kuraga jvanbruegge feel free to give any suggestions or ideas

My suggestion is to chill for a second and give the other maintainers a chance to catch up before creating more material to parse. I never said you are not allowed to comment. If you don't want suggestions or ideas, don't ask people for them.

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.

None yet

4 participants