Skip to content
This repository has been archived by the owner on Jul 6, 2021. It is now read-only.

add createContext API in next preact confing #400

Merged
merged 10 commits into from
Feb 14, 2019
Merged

Conversation

toshi1127
Copy link
Contributor

Issues, URL

abstract

Since next.js v8.0.0 does not support the createContext API, an error occurred in the preact example.
I added createContext to next-plugins / packages / next-preact / index.jswith reference to this example.

} = require('preact-compat');
const { createContext } = require('preact-context');

module.exports = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this actually be

const preact = require('preact-compat')
const {createContext} = require('preact-context')

preact.createContext = createContext

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timneutkens
with that proposal, it worked normally.

@toshi1127
Copy link
Contributor Author

@timneutkens
fixed it! Please confirm.

@timneutkens timneutkens requested a review from Timer February 14, 2019 13:21
@@ -9,7 +9,8 @@
"preact-compat": "^3.17.0"
},
"dependencies": {
"module-alias": "2.0.6"
"module-alias": "2.0.6",
"preact-context": "^1.1.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please make this a peer dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I added it to peer dependency.

@@ -1,6 +1,6 @@
const moduleAlias = require('module-alias')

module.exports = () => {
moduleAlias.addAlias('react', 'preact-compat')
moduleAlias.addAlias('react', __dirname + '/preact-compat')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add .js to the end of these? It wasn't immediately obvious to me that we had our own file named preact-compat.js instead of the module itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry, it was a bit incomprehensible.
added .js to the end of these.

@Timer
Copy link
Member

Timer commented Feb 14, 2019

Also, please update the README to reflect adding the new package as well.

review modification

Co-Authored-By: toshi1127 <32378535+toshi1127@users.noreply.github.com>
@toshi1127
Copy link
Contributor Author

@Timer
@timneutkens
fixed it! Please confirm.

Timer and others added 2 commits February 15, 2019 00:32
fix

Co-Authored-By: toshi1127 <32378535+toshi1127@users.noreply.github.com>
fix

Co-Authored-By: toshi1127 <32378535+toshi1127@users.noreply.github.com>
@toshi1127
Copy link
Contributor Author

@Timer
@timneutkens
I am sorry, my confirmation is insufficient.
Since __dirname is an absolute path, it is the same as join ...

fixed it! Please confirm.

@Timer
Copy link
Member

Timer commented Feb 14, 2019

Awesome, thank you!

@Timer Timer merged commit c1de357 into vercel:master Feb 14, 2019
@exentrich
Copy link

How to start using this? @zeit/next-preact package in NPM do not have this changes

@toshi1127
Copy link
Contributor Author

toshi1127 commented Feb 21, 2019

@Timer
@timneutkens
I'm sorry
I forgot to update package.json on next.js side.
I updated @ zeit / next-preact of package.json in the sample of next.js to the latest version 1.0.0, but it will not be reflected ... Still, this fix has not been released?

@OndeVai
Copy link

OndeVai commented Feb 25, 2019

looks like the changes are not in npm yet?

@exentrich
Copy link

Please publish this changes to NPM! :)

@myatt95
Copy link

myatt95 commented Mar 3, 2019

Any updates on this being published?

@kenk2
Copy link

kenk2 commented Mar 20, 2019

Still looks like the changes haven't been published to NPM yet, I've found in the meantime that I've had to either rollback nextjs to 7.0.3 with the current latest npm version or add these files manually if you plan to use it with nextjs 8.

@EirikFA
Copy link

EirikFA commented Mar 28, 2019

When is this going to be published to NPM?

@sievan
Copy link

sievan commented Apr 17, 2019

Still looks like this isn't published on NPM. Could a maintainer have a look at this? :)

@BrentonWatt
Copy link

Any update on an NPM publish?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants