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: export default in namespace #2200 #2267

Merged

Conversation

technohippy
Copy link
Member

fix: #2200

Now the same error message as TS is used.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

@romdotdog
Copy link
Contributor

romdotdog commented Apr 19, 2022

As mentioned in the source issue, I think changing the message for this error could be beneficial, wdyt? Do you think it's confusing? cc: @dcodeIO

@dcodeIO
Copy link
Member

dcodeIO commented Apr 19, 2022

I guess the reference to "ECMAScript-style module" can be dropped, as this is all AS supports anyway. Perhaps: "A default export can only be used in a module." Still about the same message, but less over-precise.

Another thought: I wonder that a NodeKind.EXPORT even ends up in namespace initialization. Seems brittle. Can this perhaps be checked during parsing already, not appending the export in this place?

@romdotdog
Copy link
Contributor

I was thinking something more like A default export cannot be used in a namespace. or maybe even better A default export can only be used in the top level. unless there is another case where they can be used?

@technohippy
Copy link
Member Author

TS1258: A default export must be at the top level of a file or module declaration. may be the one we want.

@romdotdog
Copy link
Contributor

I don't think AS supports default exports in modules yet, but I'm not sure if that's an upcoming feature.

@technohippy
Copy link
Member Author

hmm... I choose A default export can only be used in a module. for now. Anyone can change this anytime.

@technohippy
Copy link
Member Author

@dcodeIO Test / Bootstrap throws a strange error now. npm run bootstrap can run on my local env. Do you have any idea?

TypeError: WebAssembly.compileStreaming is not a function
    at file:///home/runner/work/assemblyscript/assemblyscript/build/assemblyscript.debug.js:2[23](https://github.com/AssemblyScript/assemblyscript/runs/6088770012?check_suite_focus=true#step:6:23)3:21
    at file:///home/runner/work/assemblyscript/assemblyscript/build/assemblyscript.debug.js:2238:3
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:[24](https://github.com/AssemblyScript/assemblyscript/runs/6088770012?check_suite_focus=true#step:6:24))
    at <anonymous> (/home/runner/work/assemblyscript/assemblyscript/cli/index.js:47:20)

@romdotdog
Copy link
Contributor

romdotdog commented Apr 20, 2022

Only one thing on the logs caught my eye: the newly released Node v18.0.0 was used as opposed to the first run which used Node v17.9.0. Needs further testing.

@dcodeIO
Copy link
Member

dcodeIO commented Apr 23, 2022

Fixed the CI error in #2274. Merging main should resolve the error in this PR as well.

@technohippy
Copy link
Member Author

@dcodeIO Thanks. Now all checkes have passed.

1 similar comment
@technohippy
Copy link
Member Author

@dcodeIO Thanks. Now all checkes have passed.

@dcodeIO dcodeIO merged commit 7dc3de0 into AssemblyScript:main Jul 23, 2022
@dcodeIO
Copy link
Member

dcodeIO commented Jul 23, 2022

Thanks!

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.

export default in namespace crashes compiler
3 participants